| Message ID | 20230128202622.12676-5-kaehndan@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | DeviceTree Support for USB-HID Devices and CP2112 | expand |
On 28/01/2023 21:26, Danny Kaehn wrote: > Bind i2c and gpio interfaces to subnodes with names > "i2c" and "gpio" if they exist, respectively. This > allows the gpio and i2c controllers to be described > in DT as usual. > > Signed-off-by: Danny Kaehn <kaehndan@gmail.com> > --- > drivers/hid/hid-cp2112.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c > index 27cadadda7c9..99e8043e1c34 100644 > --- a/drivers/hid/hid-cp2112.c > +++ b/drivers/hid/hid-cp2112.c > @@ -1310,6 +1310,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) > dev->adap.algo = &smbus_algorithm; > dev->adap.algo_data = dev; > dev->adap.dev.parent = &hdev->dev; > + dev->adap.dev.of_node = of_get_child_by_name(hdev->dev.of_node, "i2c"); > snprintf(dev->adap.name, sizeof(dev->adap.name), > "CP2112 SMBus Bridge on hidraw%d", > ((struct hidraw *)hdev->hidraw)->minor); > @@ -1336,6 +1337,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) > dev->gc.ngpio = 8; > dev->gc.can_sleep = 1; > dev->gc.parent = &hdev->dev; > +#if defined(CONFIG_OF_GPIO) Don't use #if, but IS_ENABLED(). I think it should work here. > + dev->gc.of_node = of_get_child_by_name(hdev->dev.of_node, "gpio"); You leak it now on error paths. Best regards, Krzysztof
On Sun, Jan 29, 2023 at 5:06 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 28/01/2023 21:26, Danny Kaehn wrote: > > +#if defined(CONFIG_OF_GPIO) > > Don't use #if, but IS_ENABLED(). I think it should work here. I think I will still need to use an #if / some sort of preprocessor directive, since of_node is only a member of the gpio_chip struct if that is enabled (and thus causes a compile error if done outside of the preprocessor)... Unless I'm misinterpreting your comment? > > > + dev->gc.of_node = of_get_child_by_name(hdev->dev.of_node, "gpio"); > > You leak it now on error paths. Ah, good point. Will fix! Thanks, Danny Kaehn
On 31/01/2023 02:06, Daniel Kaehn wrote: > On Sun, Jan 29, 2023 at 5:06 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 28/01/2023 21:26, Danny Kaehn wrote: >>> +#if defined(CONFIG_OF_GPIO) >> >> Don't use #if, but IS_ENABLED(). I think it should work here. > > I think I will still need to use an #if / some sort of preprocessor directive, > since of_node is only a member of the gpio_chip struct if that is enabled > (and thus causes a compile error if done outside of the preprocessor)... > Unless I'm misinterpreting your comment? If of_node in gpio_chip is indeed hidden by #ifdef, then the code is ok. Best regards, Krzysztof
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 27cadadda7c9..99e8043e1c34 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -1310,6 +1310,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) dev->adap.algo = &smbus_algorithm; dev->adap.algo_data = dev; dev->adap.dev.parent = &hdev->dev; + dev->adap.dev.of_node = of_get_child_by_name(hdev->dev.of_node, "i2c"); snprintf(dev->adap.name, sizeof(dev->adap.name), "CP2112 SMBus Bridge on hidraw%d", ((struct hidraw *)hdev->hidraw)->minor); @@ -1336,6 +1337,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) dev->gc.ngpio = 8; dev->gc.can_sleep = 1; dev->gc.parent = &hdev->dev; +#if defined(CONFIG_OF_GPIO) + dev->gc.of_node = of_get_child_by_name(hdev->dev.of_node, "gpio"); +#endif dev->irq.name = "cp2112-gpio"; dev->irq.irq_startup = cp2112_gpio_irq_startup; @@ -1391,6 +1395,11 @@ static void cp2112_remove(struct hid_device *hdev) struct cp2112_device *dev = hid_get_drvdata(hdev); int i; + of_node_put(dev->adap.dev.of_node); +#if defined(CONFIG_OF_GPIO) + of_node_put(dev->gc.of_node); +#endif + sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group); i2c_del_adapter(&dev->adap);
Bind i2c and gpio interfaces to subnodes with names "i2c" and "gpio" if they exist, respectively. This allows the gpio and i2c controllers to be described in DT as usual. Signed-off-by: Danny Kaehn <kaehndan@gmail.com> --- drivers/hid/hid-cp2112.c | 9 +++++++++ 1 file changed, 9 insertions(+)