Re: [PATCH] media: ipu3-cio2: support multiple sensors and VCMs with HID name

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Mar 06, 2023 at 01:33:30PM +0000, Cao, Bingbu wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Monday, March 6, 2023 19:27
> > On Fri, Mar 03, 2023 at 10:44:32PM +0800, bingbu.cao@intel.com wrote:

...

> > > +	if (sensor->ssdb.vcmtype) {
> > > +		scnprintf(vcm_name, sizeof(vcm_name), "%s-%u",
> > > +			  cio2_vcm_types[sensor->ssdb.vcmtype - 1],
> > > +			  sensor->ssdb.link);
> > 
> > Is using 'c' variant a cargo cult? Otherwise explain, why dropping the
> > last part of the number is not a problem.
> 
> Sorry, I can't understand. What is cargo cult?

Use of sCnprintf(). I.o.w. can you explain the point of using it instead of
simply snprintf()?

> > > +		nodes[SWNODE_VCM] = NODE_VCM(vcm_name);
> > > +	}

...

> > > +		scnprintf(sensor->name, sizeof(sensor->name), "%s-%u",
> > > +			  cfg->hid, sensor->ssdb.link);

Ditto.

...

> > > -	char name[ACPI_ID_LEN];
> > > +	char name[ACPI_ID_LEN + 4];
> > 
> > Why 4 is chosen? This needs an explanation.
> 
> 'link' is u8, so it is supposed to be max 4 characters along with '-'.

It should be mentioned somewhere.

-- 
With Best Regards,
Andy Shevchenko