Hi Bryan, Thanks for the update. On Thu, Sep 15, 2022 at 01:35:20AM +0100, Bryan O'Donoghue wrote: > imx412, imx477 and imx577 all return the same chip-id when interrogated via > i2c. I've confirmed this myself by > > - Looking at the code in Qcom and Nvidia stacks > - Running the upstream imx412 driver on imx577 with a Qcom sm8250 RB5 > - Running the downstream Qcom stack on the same hardware. This uses a > commercial licensed stack with a driver/userspace pair that make no > differentiation between imx412, imx477 and imx577. > - Running the imx412 and imx577 on a Nvidia Nano with cameras from Leopard > Imaging. Again this is a commercial non-upstream user-space/kernel-space > pairing and again the same imx driver, works for both parts. > > Sakari suggested we should add a new compat but that the compat string > should also set the media entity name also > > https://patchwork.kernel.org/project/linux-media/patch/20220607134057.2427663-3-bryan.odonoghue@xxxxxxxxxx/#24894500 > > Set up the .data parameter of of_device_id to pass a string which > we use to set the media entity name. Once done we can add in imx477 and > imx577 as compatible chips with the media names reflecting the directed > compat string. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > --- > drivers/media/i2c/imx412.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c > index a1394d6c1432..bc7fdb24235f 100644 > --- a/drivers/media/i2c/imx412.c > +++ b/drivers/media/i2c/imx412.c > @@ -1172,6 +1172,7 @@ static int imx412_init_controls(struct imx412 *imx412) > static int imx412_probe(struct i2c_client *client) > { > struct imx412 *imx412; > + const char *name; > int ret; > > imx412 = devm_kzalloc(&client->dev, sizeof(*imx412), GFP_KERNEL); > @@ -1179,6 +1180,10 @@ static int imx412_probe(struct i2c_client *client) > return -ENOMEM; > > imx412->dev = &client->dev; > + if (dev_fwnode(&client->dev)) > + name = device_get_match_data(&client->dev); No need to make this conditional. But you could return an error if name is NULL. It would most probably mean a driver bug though. > + else > + return -ENODEV; > > /* Initialize subdev */ > v4l2_i2c_subdev_init(&imx412->sd, client, &imx412_subdev_ops); > @@ -1218,6 +1223,8 @@ static int imx412_probe(struct i2c_client *client) > imx412->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > imx412->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; > > + v4l2_i2c_subdev_set_name(&imx412->sd, client, name, NULL); > + > /* Initialize source pad */ > imx412->pad.flags = MEDIA_PAD_FL_SOURCE; > ret = media_entity_pads_init(&imx412->sd.entity, 1, &imx412->pad); > @@ -1281,7 +1288,7 @@ static const struct dev_pm_ops imx412_pm_ops = { > }; > > static const struct of_device_id imx412_of_match[] = { > - { .compatible = "sony,imx412" }, > + { .compatible = "sony,imx412", .data = "imx412" }, > { } > }; > -- Kind regards, Sakari Ailus
