| Message ID | 20220513190409.3682501-1-kyle.swenson@est.tech |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] leds: aw21024: Add support for Awinic's AW21024 | expand |
On Fri, 13 May 2022 13:04:09 -0600, Kyle Swenson wrote: > Add device-tree bindings for Awinic's aw21024 24 channel RGB LED Driver. > > Datasheet: > https://www.awinic.com/Public/Uploads/uploadfile/files/20200511/20200511165751_5eb9138fcd9e3.PDF > > Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> > --- > .../bindings/leds/leds-aw21024.yaml | 157 ++++++++++++++++++ > 1 file changed, 157 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-aw21024.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/leds/leds-aw21024.yaml:54:1: [error] duplication of key "patternProperties" in mapping (key-duplicates) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/leds/leds-aw21024.example.dts' Documentation/devicetree/bindings/leds/leds-aw21024.yaml: found duplicate key "patternProperties" with value "{}" (original value: "{}") make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/leds/leds-aw21024.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... Traceback (most recent call last): File "/usr/local/bin/dt-doc-validate", line 25, in check_doc testtree = dtschema.load(filename, line_number=line_number) File "/usr/local/lib/python3.10/dist-packages/dtschema/lib.py", line 914, in load return yaml.load(f.read()) File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/main.py", line 434, in load return constructor.get_single_data() File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 121, in get_single_data return self.construct_document(node) File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 131, in construct_document for _dummy in generator: File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 674, in construct_yaml_map value = self.construct_mapping(node) File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 445, in construct_mapping return BaseConstructor.construct_mapping(self, node, deep=deep) File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 263, in construct_mapping if self.check_mapping_key(node, key_node, mapping, key, value): File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/constructor.py", line 294, in check_mapping_key raise DuplicateKeyError(*args) ruamel.yaml.constructor.DuplicateKeyError: while constructing a mapping in "<unicode string>", line 4, column 1 found duplicate key "patternProperties" with value "{}" (original value: "{}") in "<unicode string>", line 54, column 1 To suppress this check see: http://yaml.readthedocs.io/en/latest/api.html#duplicate-keys During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/local/bin/dt-doc-validate", line 74, in <module> ret = check_doc(f) File "/usr/local/bin/dt-doc-validate", line 30, in check_doc print(filename + ":", exc.path[-1], exc.message, file=sys.stderr) AttributeError: 'DuplicateKeyError' object has no attribute 'path' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-aw21024.yaml: ignoring, error parsing file make: *** [Makefile:1401: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Hi Kyle,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pavel-leds/for-next]
[also build test ERROR on robh/for-next v5.18-rc7 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: mips-randconfig-r006-20220516 (https://download.01.org/0day-ci/archive/20220516/202205161802.8WYsbizM-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/38eeda60299918b5599f4a58714dc91f9741677c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
git checkout 38eeda60299918b5599f4a58714dc91f9741677c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/leds/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/leds/leds-aw21024.c: In function 'aw21024_led_brightness_set':
>> drivers/leds/leds-aw21024.c:64:31: error: implicit declaration of function 'i2c_smbus_write_byte_data' [-Werror=implicit-function-declaration]
64 | ret = i2c_smbus_write_byte_data(parent->client,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/leds/leds-aw21024.c: In function 'aw21024_configure':
>> drivers/leds/leds-aw21024.c:213:15: error: implicit declaration of function 'i2c_smbus_read_byte_data' [-Werror=implicit-function-declaration]
213 | ret = i2c_smbus_read_byte_data(client, AW21024_REG_SW_RESET);
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/leds/leds-aw21024.c: At top level:
>> drivers/leds/leds-aw21024.c:310:1: warning: data definition has no type or storage class
310 | module_i2c_driver(aw21024_driver);
| ^~~~~~~~~~~~~~~~~
>> drivers/leds/leds-aw21024.c:310:1: error: type defaults to 'int' in declaration of 'module_i2c_driver' [-Werror=implicit-int]
>> drivers/leds/leds-aw21024.c:310:1: warning: parameter names (without types) in function declaration
drivers/leds/leds-aw21024.c:301:26: warning: 'aw21024_driver' defined but not used [-Wunused-variable]
301 | static struct i2c_driver aw21024_driver = {
| ^~~~~~~~~~~~~~
drivers/leds/leds-aw21024.c:295:34: warning: 'of_aw21024_leds_match' defined but not used [-Wunused-const-variable=]
295 | static const struct of_device_id of_aw21024_leds_match[] = {
| ^~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/i2c_smbus_write_byte_data +64 drivers/leds/leds-aw21024.c
51
52 static int aw21024_led_brightness_set(struct led_classdev *led_cdev,
53 enum led_brightness brightness)
54 {
55 struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
56 struct aw21024_led_data *led = container_of(mc_cdev, struct aw21024_led_data, mc_cdev);
57 struct aw21024 *parent = led->parent;
58 int i;
59 int ret = 0;
60
61 mutex_lock(&parent->lock);
62 if (mc_cdev->num_colors && mc_cdev->subled_info) {
63 for (i = 0; i < led->nregs; i++) {
> 64 ret = i2c_smbus_write_byte_data(parent->client,
65 AW21024_REG_DC_CURRENT(led->regs[i]),
66 mc_cdev->subled_info[i].intensity);
67 if (ret < 0)
68 goto unlock_ret;
69
70 ret = i2c_smbus_write_byte_data(parent->client,
71 AW21024_REG_BRIGHTNESS(led->regs[i]),
72 brightness);
73 if (ret < 0)
74 goto unlock_ret;
75 }
76 } else {
77 ret = i2c_smbus_write_byte_data(parent->client,
78 AW21024_REG_DC_CURRENT(led->regs[0]), 0xFF);
79 if (ret < 0)
80 goto unlock_ret;
81
82 ret = i2c_smbus_write_byte_data(parent->client,
83 AW21024_REG_BRIGHTNESS(led->regs[0]),
84 brightness);
85 if (ret < 0)
86 goto unlock_ret;
87 }
88 ret = i2c_smbus_write_byte_data(parent->client, AW21024_REG_UPDATE, 0x0);
89 unlock_ret:
90 mutex_unlock(&parent->lock);
91 return ret;
92 }
93
94 static int aw21024_probe_dt(struct aw21024 *data)
95 {
96 struct device *dev = &data->client->dev;
97 struct fwnode_handle *child = NULL;
98 struct fwnode_handle *led_node = NULL;
99 struct led_init_data init_data = {};
100 u32 color_id;
101 int ret, num_colors;
102 unsigned int nleds = 0;
103 struct aw21024_led_data *led;
104 struct led_classdev *led_cdev;
105 struct mc_subled *mc_led_info;
106
107 nleds = device_get_child_node_count(dev);
108
109 data->leds = devm_kcalloc(dev, nleds, sizeof(*(data->leds)), GFP_KERNEL);
110 if (!data->leds)
111 return -ENOMEM;
112
113 device_for_each_child_node(dev, child) {
114 led = devm_kzalloc(dev, sizeof(struct aw21024_led_data), GFP_KERNEL);
115 if (!led) {
116 ret = -ENOMEM;
117 goto ret_put_child;
118 }
119 led->parent = data;
120 led_cdev = &led->mc_cdev.led_cdev;
121 init_data.fwnode = child;
122
123 led_cdev->brightness_set_blocking = aw21024_led_brightness_set;
124 data->leds[data->nleds] = led;
125
126 ret = fwnode_property_count_u32(child, "reg");
127 if (ret < 0) {
128 dev_err(dev, "reg property is invalid in node %s\n",
129 fwnode_get_name(child));
130 goto ret_put_child;
131 }
132
133 led->regs = devm_kcalloc(dev, ret, sizeof(*(led->regs)), GFP_KERNEL);
134 led->nregs = ret;
135 if (!led->regs) {
136 ret = -ENOMEM;
137 goto ret_put_child;
138 }
139
140 ret = fwnode_property_read_u32_array(child, "reg", led->regs, led->nregs);
141 if (ret) {
142 dev_err(dev, "Failed to read reg array, error=%d\n", ret);
143 goto ret_put_child;
144 }
145
146 if (led->nregs > 1) {
147 mc_led_info = devm_kcalloc(dev, led->nregs,
148 sizeof(*mc_led_info), GFP_KERNEL);
149 if (!mc_led_info) {
150 ret = -ENOMEM;
151 goto ret_put_child;
152 }
153
154 num_colors = 0;
155 fwnode_for_each_child_node(child, led_node) {
156 if (num_colors > led->nregs) {
157 ret = -EINVAL;
158 fwnode_handle_put(led_node);
159 goto ret_put_child;
160 }
161
162 ret = fwnode_property_read_u32(led_node, "color",
163 &color_id);
164 if (ret) {
165 fwnode_handle_put(led_node);
166 goto ret_put_child;
167 }
168 mc_led_info[num_colors].color_index = color_id;
169 num_colors++;
170 }
171
172 led->mc_cdev.num_colors = num_colors;
173 led->mc_cdev.subled_info = mc_led_info;
174 ret = devm_led_classdev_multicolor_register_ext(dev,
175 &led->mc_cdev,
176 &init_data);
177 if (ret < 0) {
178 dev_warn(dev, "Failed to register multicolor LED %s, err=%d\n",
179 fwnode_get_name(child), ret);
180 goto ret_put_child;
181 }
182 } else {
183 ret = devm_led_classdev_register_ext(dev,
184 &led->mc_cdev.led_cdev, &init_data);
185 if (ret < 0) {
186 dev_warn(dev, "Failed to register LED %s, err=%d\n",
187 fwnode_get_name(child), ret);
188 goto ret_put_child;
189 }
190 }
191 data->nleds++;
192 }
193
194 return 0;
195
196 ret_put_child:
197 fwnode_handle_put(child);
198 return ret;
199 }
200
201 /* Expected to be called prior to registering with the LEDs class */
202 static int aw21024_configure(struct aw21024 *priv)
203 {
204 int ret = 0;
205 struct i2c_client *client = priv->client;
206
207 ret = i2c_smbus_write_byte_data(client, AW21024_REG_GCR0, AW21024_GCR0_CHIPEN);
208 if (ret < 0) {
209 dev_err(&client->dev, "Failed to write chip enable\n");
210 return -ENODEV;
211 }
212
> 213 ret = i2c_smbus_read_byte_data(client, AW21024_REG_SW_RESET);
214 if (ret < 0) {
215 dev_err(&client->dev, "Failed to read chip id\n");
216 return -ENODEV;
217 }
218
219 if (ret != AW21024_CHIP_ID) {
220 dev_err(&client->dev, "Chip ID 0x%02X doesn't match expected (0x%02X)\n",
221 ret, AW21024_CHIP_ID);
222 return -ENODEV;
223 }
224
225 ret = i2c_smbus_read_byte_data(client, AW21024_REG_VERSION);
226 if (ret < 0) {
227 dev_err(&client->dev, "Failed to read chip version\n");
228 return -ENODEV;
229 }
230 if (ret != AW21024_CHIP_VERSION)
231 dev_warn(&client->dev, "Chip version 0x%02X doesn't match expected 0x%02X\n",
232 ret, AW21024_CHIP_VERSION);
233
234 i2c_smbus_write_byte_data(client, AW21024_REG_SW_RESET, 0x00);
235 mdelay(2);
236 i2c_smbus_write_byte_data(client, AW21024_REG_GCR0, AW21024_GCR0_CHIPEN);
237 i2c_smbus_write_byte_data(client, AW21024_REG_GCC, 0xFF);
238
239 return 0;
240 }
241
Hi Kyle,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pavel-leds/for-next]
[also build test WARNING on robh/for-next v5.18-rc7 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: x86_64-randconfig-r011-20220516 (https://download.01.org/0day-ci/archive/20220516/202205161859.bDAFU09i-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/38eeda60299918b5599f4a58714dc91f9741677c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
git checkout 38eeda60299918b5599f4a58714dc91f9741677c
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/leds/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/leds/leds-aw21024.c:295:34: warning: 'of_aw21024_leds_match' defined but not used [-Wunused-const-variable=]
295 | static const struct of_device_id of_aw21024_leds_match[] = {
| ^~~~~~~~~~~~~~~~~~~~~
vim +/of_aw21024_leds_match +295 drivers/leds/leds-aw21024.c
294
> 295 static const struct of_device_id of_aw21024_leds_match[] = {
296 { .compatible = "awinic,aw21024", },
297 {},
298 };
299 MODULE_DEVICE_TABLE(of, of_aw21024_leds_match);
300
Hi Kyle,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pavel-leds/for-next]
[also build test WARNING on robh/for-next v5.18-rc7 next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
base: git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
config: hexagon-randconfig-r014-20220516 (https://download.01.org/0day-ci/archive/20220516/202205162025.GEAQ50Y7-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/38eeda60299918b5599f4a58714dc91f9741677c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kyle-Swenson/leds-aw21024-Add-support-for-Awinic-s-AW21024/20220514-030705
git checkout 38eeda60299918b5599f4a58714dc91f9741677c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/leds/leds-aw21024.c:295:34: warning: unused variable 'of_aw21024_leds_match' [-Wunused-const-variable]
static const struct of_device_id of_aw21024_leds_match[] = {
^
1 warning generated.
vim +/of_aw21024_leds_match +295 drivers/leds/leds-aw21024.c
294
> 295 static const struct of_device_id of_aw21024_leds_match[] = {
296 { .compatible = "awinic,aw21024", },
297 {},
298 };
299 MODULE_DEVICE_TABLE(of, of_aw21024_leds_match);
300
On 13/05/2022 21:04, Kyle Swenson wrote: > The Awinic AW21024 LED controller is a 24-channel RGB LED controller. > Each LED on the controller can be controlled individually or grouped > with other LEDs on the controller to form a multi-color LED. Arbitrary > combinations of individual and grouped LED control should be possible. > > Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> Thank you for your patch. There is something to discuss/improve. > + > +static const struct i2c_device_id aw21024_id[] = { > + { "aw21024", 0 }, /* 24 Channel */ > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, aw21024_id); > + > +static const struct of_device_id of_aw21024_leds_match[] = { > + { .compatible = "awinic,aw21024", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match); > + > +static struct i2c_driver aw21024_driver = { > + .driver = { > + .name = "aw21024", > + .of_match_table = of_match_ptr(of_aw21024_leds_match), of_match_ptr causes this being unused. kbuild robot probably pointed this out... if not - of_match_ptr goes with maybe_unused. You need both or none, depending on intended usage. > + }, > + .probe_new = aw21024_probe, > + .remove = aw21024_remove, > + .id_table = aw21024_id, Why other places are indented but this not? > +}; > +module_i2c_driver(aw21024_driver); > + > +MODULE_AUTHOR("Kyle Swenson <kyle.swenson@est.tech>"); > +MODULE_DESCRIPTION("Awinic AW21024 LED driver"); > +MODULE_LICENSE("GPL"); Best regards, Krzysztof
On Tue, May 17, 2022 at 11:11:37AM +0200, Krzysztof Kozlowski wrote: > On 13/05/2022 21:04, Kyle Swenson wrote: > > The Awinic AW21024 LED controller is a 24-channel RGB LED controller. > > Each LED on the controller can be controlled individually or grouped > > with other LEDs on the controller to form a multi-color LED. Arbitrary > > combinations of individual and grouped LED control should be possible. > > > > Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> > > Thank you for your patch. There is something to discuss/improve. > > > + > > +static const struct i2c_device_id aw21024_id[] = { > > + { "aw21024", 0 }, /* 24 Channel */ > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, aw21024_id); > > + > > +static const struct of_device_id of_aw21024_leds_match[] = { > > + { .compatible = "awinic,aw21024", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match); > > + > > +static struct i2c_driver aw21024_driver = { > > + .driver = { > > + .name = "aw21024", > > + .of_match_table = of_match_ptr(of_aw21024_leds_match), > > of_match_ptr causes this being unused. kbuild robot probably pointed > this out... if not - of_match_ptr goes with maybe_unused. You need both > or none, depending on intended usage. > Ah, yes, the kbuild robot did point this out to me, and I had planned on fixing by adding 'depends on OF' to the Kconfig. Perhaps that isn't correct or complete (or even relevant)? I'll do some investigating and determine if I need to use of_match_ptr or not and I'll fix it either by removing it or adding maybe_unused in the next version. > > + }, > > + .probe_new = aw21024_probe, > > + .remove = aw21024_remove, > > + .id_table = aw21024_id, > > Why other places are indented but this not? Sorry, it should be. My editor was configured wrong and this now looks bad. There are a few other places in the driver that also now look bad and I'll fix those before submitting v2. > > > > +}; > > +module_i2c_driver(aw21024_driver); > > + > > +MODULE_AUTHOR("Kyle Swenson <kyle.swenson@est.tech>"); > > +MODULE_DESCRIPTION("Awinic AW21024 LED driver"); > > +MODULE_LICENSE("GPL"); > > > Best regards, > Krzysztof Thanks so much for taking a look at this, I really appreciate your time and patience. Thanks, Kyle
On 17/05/2022 20:36, Kyle Swenson wrote: >>> +static const struct of_device_id of_aw21024_leds_match[] = { >>> + { .compatible = "awinic,aw21024", }, >>> + {}, >>> +}; >>> +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match); >>> + >>> +static struct i2c_driver aw21024_driver = { >>> + .driver = { >>> + .name = "aw21024", >>> + .of_match_table = of_match_ptr(of_aw21024_leds_match), >> >> of_match_ptr causes this being unused. kbuild robot probably pointed >> this out... if not - of_match_ptr goes with maybe_unused. You need both >> or none, depending on intended usage. >> > Ah, yes, the kbuild robot did point this out to me, and I had planned on > fixing by adding 'depends on OF' to the Kconfig. Perhaps that isn't > correct or complete (or even relevant)? > > I'll do some investigating and determine if I need to use of_match_ptr > or not and I'll fix it either by removing it or adding maybe_unused in > the next version. Your has i2c_device_id so it could bind without OF, however obviously aw21024_probe_dt() will do nothing and return 0. Therefore it is up to you if you want to add dependency on OF. If you add, please add it with "|| COMPILE_TEST". Then in both cases you need to handle the case of building (not running) a driver without OF: using maybe_unused+of_match_ptr() or nothing (thus always referencing of_device_id). Which one to choose matters less. Using it causes the code to be smaller for !OF case, which might matter for some distros which build everything as module. Not using it allows to match the driver on ACPI systems, although I am not sure if this is relevant. I don't have recommendation on that - just be sure there are no warnings. Best regards, Krzysztof
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a49979f41eee..c813d7c16ff8 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -102,10 +102,21 @@ config LEDS_AW2013 LED driver. To compile this driver as a module, choose M here: the module will be called leds-aw2013. +config LEDS_AW21024 + tristate "LED Support for Awinic AW21024" + depends on LEDS_CLASS + depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR + help + If you say yes here you get support for Awinic's AW21024, a 24-channel + RGB LED Driver. + + To compile this driver as a module, choose M here: the + module will be called leds-aw21024. + config LEDS_BCM6328 tristate "LED Support for Broadcom BCM6328" depends on LEDS_CLASS depends on HAS_IOMEM depends on OF diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 4fd2f92cd198..09a0e3cb21cb 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -14,10 +14,11 @@ obj-$(CONFIG_LEDS_ADP5520) += leds-adp5520.o obj-$(CONFIG_LEDS_AN30259A) += leds-an30259a.o obj-$(CONFIG_LEDS_APU) += leds-apu.o obj-$(CONFIG_LEDS_ARIEL) += leds-ariel.o obj-$(CONFIG_LEDS_ASIC3) += leds-asic3.o obj-$(CONFIG_LEDS_AW2013) += leds-aw2013.o +obj-$(CONFIG_LEDS_AW21024) += leds-aw21024.o obj-$(CONFIG_LEDS_BCM6328) += leds-bcm6328.o obj-$(CONFIG_LEDS_BCM6358) += leds-bcm6358.o obj-$(CONFIG_LEDS_BD2802) += leds-bd2802.o obj-$(CONFIG_LEDS_BLINKM) += leds-blinkm.o obj-$(CONFIG_LEDS_CLEVO_MAIL) += leds-clevo-mail.o diff --git a/drivers/leds/leds-aw21024.c b/drivers/leds/leds-aw21024.c new file mode 100644 index 000000000000..4eebc3de1a8a --- /dev/null +++ b/drivers/leds/leds-aw21024.c @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: GPL-2.0 +// Awinic AW21024 LED chip driver +// Copyright (C) 2022 Nordix Foundation https://www.nordix.org + +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/leds.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/delay.h> +#include <linux/slab.h> +#include <uapi/linux/uleds.h> + +#include <linux/led-class-multicolor.h> + +/* Called COL0, COL1,..., COL23 in datasheet */ +#define AW21024_REG_DC_CURRENT(_led) (0x4a + (_led)) + +/* Called BR0, BR1,..., BR23 in datasheet */ +#define AW21024_REG_BRIGHTNESS(_led) (0x01 + (_led)) + +#define AW21024_REG_UPDATE 0x49 /* Write 0x00 to update BR */ + +#define AW21024_REG_GCR0 0x00 /* Global configuration register */ +#define AW21024_REG_GCC 0x6e /* Global current control */ +#define AW21024_REG_SW_RESET 0x7f +#define AW21024_REG_VERSION 0x7e + +#define AW21024_GCR0_CHIPEN BIT(0) +#define AW21024_CHIP_ID 0x18 +#define AW21024_CHIP_VERSION 0xA8 + +struct aw21024_led_data { + struct led_classdev_mc mc_cdev; + struct work_struct work; + unsigned int *regs; + unsigned int nregs; + struct aw21024 *parent; +}; + +struct aw21024 { + struct i2c_client *client; + struct device *dev; + struct gpio_desc *enable_gpio; + struct mutex lock; + struct aw21024_led_data **leds; + unsigned int nleds; +}; + +static int aw21024_led_brightness_set(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev); + struct aw21024_led_data *led = container_of(mc_cdev, struct aw21024_led_data, mc_cdev); + struct aw21024 *parent = led->parent; + int i; + int ret = 0; + + mutex_lock(&parent->lock); + if (mc_cdev->num_colors && mc_cdev->subled_info) { + for (i = 0; i < led->nregs; i++) { + ret = i2c_smbus_write_byte_data(parent->client, + AW21024_REG_DC_CURRENT(led->regs[i]), + mc_cdev->subled_info[i].intensity); + if (ret < 0) + goto unlock_ret; + + ret = i2c_smbus_write_byte_data(parent->client, + AW21024_REG_BRIGHTNESS(led->regs[i]), + brightness); + if (ret < 0) + goto unlock_ret; + } + } else { + ret = i2c_smbus_write_byte_data(parent->client, + AW21024_REG_DC_CURRENT(led->regs[0]), 0xFF); + if (ret < 0) + goto unlock_ret; + + ret = i2c_smbus_write_byte_data(parent->client, + AW21024_REG_BRIGHTNESS(led->regs[0]), + brightness); + if (ret < 0) + goto unlock_ret; + } + ret = i2c_smbus_write_byte_data(parent->client, AW21024_REG_UPDATE, 0x0); +unlock_ret: + mutex_unlock(&parent->lock); + return ret; +} + +static int aw21024_probe_dt(struct aw21024 *data) +{ + struct device *dev = &data->client->dev; + struct fwnode_handle *child = NULL; + struct fwnode_handle *led_node = NULL; + struct led_init_data init_data = {}; + u32 color_id; + int ret, num_colors; + unsigned int nleds = 0; + struct aw21024_led_data *led; + struct led_classdev *led_cdev; + struct mc_subled *mc_led_info; + + nleds = device_get_child_node_count(dev); + + data->leds = devm_kcalloc(dev, nleds, sizeof(*(data->leds)), GFP_KERNEL); + if (!data->leds) + return -ENOMEM; + + device_for_each_child_node(dev, child) { + led = devm_kzalloc(dev, sizeof(struct aw21024_led_data), GFP_KERNEL); + if (!led) { + ret = -ENOMEM; + goto ret_put_child; + } + led->parent = data; + led_cdev = &led->mc_cdev.led_cdev; + init_data.fwnode = child; + + led_cdev->brightness_set_blocking = aw21024_led_brightness_set; + data->leds[data->nleds] = led; + + ret = fwnode_property_count_u32(child, "reg"); + if (ret < 0) { + dev_err(dev, "reg property is invalid in node %s\n", + fwnode_get_name(child)); + goto ret_put_child; + } + + led->regs = devm_kcalloc(dev, ret, sizeof(*(led->regs)), GFP_KERNEL); + led->nregs = ret; + if (!led->regs) { + ret = -ENOMEM; + goto ret_put_child; + } + + ret = fwnode_property_read_u32_array(child, "reg", led->regs, led->nregs); + if (ret) { + dev_err(dev, "Failed to read reg array, error=%d\n", ret); + goto ret_put_child; + } + + if (led->nregs > 1) { + mc_led_info = devm_kcalloc(dev, led->nregs, + sizeof(*mc_led_info), GFP_KERNEL); + if (!mc_led_info) { + ret = -ENOMEM; + goto ret_put_child; + } + + num_colors = 0; + fwnode_for_each_child_node(child, led_node) { + if (num_colors > led->nregs) { + ret = -EINVAL; + fwnode_handle_put(led_node); + goto ret_put_child; + } + + ret = fwnode_property_read_u32(led_node, "color", + &color_id); + if (ret) { + fwnode_handle_put(led_node); + goto ret_put_child; + } + mc_led_info[num_colors].color_index = color_id; + num_colors++; + } + + led->mc_cdev.num_colors = num_colors; + led->mc_cdev.subled_info = mc_led_info; + ret = devm_led_classdev_multicolor_register_ext(dev, + &led->mc_cdev, + &init_data); + if (ret < 0) { + dev_warn(dev, "Failed to register multicolor LED %s, err=%d\n", + fwnode_get_name(child), ret); + goto ret_put_child; + } + } else { + ret = devm_led_classdev_register_ext(dev, + &led->mc_cdev.led_cdev, &init_data); + if (ret < 0) { + dev_warn(dev, "Failed to register LED %s, err=%d\n", + fwnode_get_name(child), ret); + goto ret_put_child; + } + } + data->nleds++; + } + + return 0; + +ret_put_child: + fwnode_handle_put(child); + return ret; +} + +/* Expected to be called prior to registering with the LEDs class */ +static int aw21024_configure(struct aw21024 *priv) +{ + int ret = 0; + struct i2c_client *client = priv->client; + + ret = i2c_smbus_write_byte_data(client, AW21024_REG_GCR0, AW21024_GCR0_CHIPEN); + if (ret < 0) { + dev_err(&client->dev, "Failed to write chip enable\n"); + return -ENODEV; + } + + ret = i2c_smbus_read_byte_data(client, AW21024_REG_SW_RESET); + if (ret < 0) { + dev_err(&client->dev, "Failed to read chip id\n"); + return -ENODEV; + } + + if (ret != AW21024_CHIP_ID) { + dev_err(&client->dev, "Chip ID 0x%02X doesn't match expected (0x%02X)\n", + ret, AW21024_CHIP_ID); + return -ENODEV; + } + + ret = i2c_smbus_read_byte_data(client, AW21024_REG_VERSION); + if (ret < 0) { + dev_err(&client->dev, "Failed to read chip version\n"); + return -ENODEV; + } + if (ret != AW21024_CHIP_VERSION) + dev_warn(&client->dev, "Chip version 0x%02X doesn't match expected 0x%02X\n", + ret, AW21024_CHIP_VERSION); + + i2c_smbus_write_byte_data(client, AW21024_REG_SW_RESET, 0x00); + mdelay(2); + i2c_smbus_write_byte_data(client, AW21024_REG_GCR0, AW21024_GCR0_CHIPEN); + i2c_smbus_write_byte_data(client, AW21024_REG_GCC, 0xFF); + + return 0; +} + +static int aw21024_probe(struct i2c_client *client) +{ + struct aw21024 *priv; + int ret; + + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->client = client; + priv->dev = &client->dev; + + mutex_init(&priv->lock); + + priv->enable_gpio = devm_gpiod_get_optional(priv->dev, "enable", GPIOD_OUT_LOW); + if (IS_ERR(priv->enable_gpio)) + return dev_err_probe(priv->dev, PTR_ERR(priv->enable_gpio), + "Failed to get enable GPIO\n"); + + if (priv->enable_gpio) { + mdelay(1); + gpiod_direction_output(priv->enable_gpio, 1); + mdelay(1); + } + + i2c_set_clientdata(client, priv); + + ret = aw21024_configure(priv); + if (ret < 0) + return ret; + + return aw21024_probe_dt(priv); +} + +static int aw21024_remove(struct i2c_client *client) +{ + struct aw21024 *priv = i2c_get_clientdata(client); + int ret; + + ret = gpiod_direction_output(priv->enable_gpio, 0); + if (ret) + dev_err(priv->dev, "Failed to disable chip, err=%d\n", ret); + + mutex_destroy(&priv->lock); + return 0; +} + +static const struct i2c_device_id aw21024_id[] = { + { "aw21024", 0 }, /* 24 Channel */ + { } +}; +MODULE_DEVICE_TABLE(i2c, aw21024_id); + +static const struct of_device_id of_aw21024_leds_match[] = { + { .compatible = "awinic,aw21024", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_aw21024_leds_match); + +static struct i2c_driver aw21024_driver = { + .driver = { + .name = "aw21024", + .of_match_table = of_match_ptr(of_aw21024_leds_match), + }, + .probe_new = aw21024_probe, + .remove = aw21024_remove, + .id_table = aw21024_id, +}; +module_i2c_driver(aw21024_driver); + +MODULE_AUTHOR("Kyle Swenson <kyle.swenson@est.tech>"); +MODULE_DESCRIPTION("Awinic AW21024 LED driver"); +MODULE_LICENSE("GPL");
The Awinic AW21024 LED controller is a 24-channel RGB LED controller. Each LED on the controller can be controlled individually or grouped with other LEDs on the controller to form a multi-color LED. Arbitrary combinations of individual and grouped LED control should be possible. Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> --- drivers/leds/Kconfig | 11 ++ drivers/leds/Makefile | 1 + drivers/leds/leds-aw21024.c | 314 ++++++++++++++++++++++++++++++++++++ 3 files changed, 326 insertions(+) create mode 100644 drivers/leds/leds-aw21024.c