Hi Laurent, CC GregKH On Fri, Oct 18, 2024 at 3:10 PM Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > On Fri, Oct 18, 2024 at 09:45:52AM +0200, Geert Uytterhoeven wrote: > > Each bridge instance creates up to four auxiliary devices with different > > names. However, their IDs are always zero, causing duplicate filename > > errors when a system has multiple bridges: > > > > sysfs: cannot create duplicate filename '/bus/auxiliary/devices/ti_sn65dsi86.gpio.0' > > > > Fix this by using a unique instance ID per bridge instance. > > Isn't this something that should be handled by the AUX core ? The code > below would otherwise need to be duplicated by all drivers, which seems > a burden we should avoid. According to the documentation, this is the responsibility of the caller https://elixir.bootlin.com/linux/v6.11.4/source/include/linux/auxiliary_bus.h#L81 I believe this is the same for platform devices. See also the example at https://elixir.bootlin.com/linux/v6.11.4/source/include/linux/auxiliary_bus.h#L116 Note: the platform bus supports PLATFORM_DEVID_AUTO, but the auxiliary bus does not. > > Fixes: bf73537f411b0d4f ("drm/bridge: ti-sn65dsi86: Break GPIO and MIPI-to-eDP bridge into sub-drivers") > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > --- > > /sys/bus/auxiliary/devices > > ├── ti_sn65dsi86.gpio.0 > > ├── ti_sn65dsi86.pwm.0 > > ├── ti_sn65dsi86.aux.0 > > ├── ti_sn65dsi86.bridge.0 > > ├── ti_sn65dsi86.gpio.1 > > ├── ti_sn65dsi86.pwm.1 > > ├── ti_sn65dsi86.aux.1 > > └── ti_sn65dsi86.bridge.1 > > --- > > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > index 9e31f750fd889745..8f6ac48aefdb70b3 100644 > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > > @@ -13,6 +13,7 @@ > > #include <linux/gpio/consumer.h> > > #include <linux/gpio/driver.h> > > #include <linux/i2c.h> > > +#include <linux/idr.h> > > #include <linux/iopoll.h> > > #include <linux/module.h> > > #include <linux/of_graph.h> > > @@ -168,6 +169,7 @@ > > * @pwm_enabled: Used to track if the PWM signal is currently enabled. > > * @pwm_pin_busy: Track if GPIO4 is currently requested for GPIO or PWM. > > * @pwm_refclk_freq: Cache for the reference clock input to the PWM. > > + * @id: Unique instance ID > > */ > > struct ti_sn65dsi86 { > > struct auxiliary_device *bridge_aux; > > @@ -202,8 +204,11 @@ struct ti_sn65dsi86 { > > atomic_t pwm_pin_busy; > > #endif > > unsigned int pwm_refclk_freq; > > + int id; > > }; > > > > +static DEFINE_IDA(ti_sn65dsi86_ida); > > + > > static const struct regmap_range ti_sn65dsi86_volatile_ranges[] = { > > { .range_min = 0, .range_max = 0xFF }, > > }; > > @@ -488,6 +493,7 @@ static int ti_sn65dsi86_add_aux_device(struct ti_sn65dsi86 *pdata, > > return -ENOMEM; > > > > aux->name = name; > > + aux->id = pdata->id; > > aux->dev.parent = dev; > > aux->dev.release = ti_sn65dsi86_aux_device_release; > > device_set_of_node_from_dev(&aux->dev, dev); > > @@ -1889,6 +1895,13 @@ static int ti_sn65dsi86_parse_regulators(struct ti_sn65dsi86 *pdata) > > pdata->supplies); > > } > > > > +static void ti_sn65dsi86_devm_ida_free(void *data) > > +{ > > + struct ti_sn65dsi86 *pdata = data; > > + > > + ida_free(&ti_sn65dsi86_ida, pdata->id); > > +} > > + > > static int ti_sn65dsi86_probe(struct i2c_client *client) > > { > > struct device *dev = &client->dev; > > @@ -1903,6 +1916,17 @@ static int ti_sn65dsi86_probe(struct i2c_client *client) > > pdata = devm_kzalloc(dev, sizeof(struct ti_sn65dsi86), GFP_KERNEL); > > if (!pdata) > > return -ENOMEM; > > + > > + ret = ida_alloc(&ti_sn65dsi86_ida, GFP_KERNEL); > > + if (ret < 0) > > + return ret; > > + > > + pdata->id = ret; > > + > > + ret = devm_add_action_or_reset(dev, ti_sn65dsi86_devm_ida_free, pdata); > > + if (ret) > > + return ret; > > + > > dev_set_drvdata(dev, pdata); > > pdata->dev = dev; > > Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
