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

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

On Fri, Mar 03, 2023 at 10:44:32PM +0800, bingbu.cao@intel.com wrote:
> From: Bingbu Cao <bingbu.cao@intel.com>
> 
> In current cio2-bridge, it is using the hid as node name to register
> software node and swnode will create kobject and sysfs entry with
> the node name, if there are multiple sensors and VCMs which are sharing
> same HID name, it will cause the software nodes registration failure:
> 
> [ 7.142311] sysfs: cannot create duplicate filename '/kernel/software_nodes/dw9714'
> ...
> [ 7.142328] Call Trace:
> [ 7.142330]  <TASK>
> [ 7.142336]  dump_stack_lvl+0x49/0x63
> [ 7.142341]  dump_stack+0x10/0x16
> [ 7.142343]  sysfs_warn_dup.cold+0x17/0x2b
> [ 7.142346]  sysfs_create_dir_ns+0xbc/0xd0
> [ 7.142351]  kobject_add_internal+0xb1/0x2b0
> [ 7.142356]  kobject_init_and_add+0x71/0xa0
> [ 7.142360]  swnode_register+0x136/0x210
> [ 7.142363]  software_node_register+0xd2/0x120
> [ 7.142364]  software_node_register_nodes+0x83/0xf0
> [ 7.142366]  ? acpi_get_physical_device_location+0x65/0xc0
> [ 7.142371]  cio2_bridge_init+0x82a/0xb5e
> ...
> [ 7.142448] kobject_add_internal failed for dw9714 with -EEXIST,
> don't try to register things with the same name in the same directory.

Please cut unneeded context of the backtrace as it's explained in the
Submitting Patches documentation.

> One solution is appending the sensor link(Mipi Port) in SSDB as suffix
> of the node name to fix this problem.

...

> +	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.

> +		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.

-- 
With Best Regards,
Andy Shevchenko