| Message ID | 20230630110643.209761-3-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: ipu-bridge: Shared with atomisp, rework VCM instantiation | expand |
Hi Hans On 30/06/2023 13:06, Hans de Goede wrote: > Commit 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs > with same HID") introduced an on stack vcm_name and then uses this for > the name field of the software_node struct used for the vcm. > > But the software_node struct is much longer lived then the current > stack-frame, so this is no good. > > Instead extend the ipu_node_names struct with an extra field to store > the vcm software_node name and use that. > > Note this also changes the length of the allocated buffer from > ACPI_ID_LEN + 4 to 16. the name is filled with "<ipu_vcm_types[x]>-%u" > where ipu_vcm_types[x] is not an ACPI_ID. The maximum length of > the strings in the ipu_vcm_types[] array is 11 + 5 bytes for "-255\0" > means 16 bytes are needed in the worst case scenario. > > Fixes: 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs with same HID") > Cc: Bingbu Cao <bingbu.cao@intel.com> > Reviewed-by: Andy Shevchenko <andy@kernel.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > drivers/media/pci/intel/ipu-bridge.c | 7 +++---- > drivers/media/pci/intel/ipu-bridge.h | 1 + > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c > index f0927f80184d..ef6c6cb7b51b 100644 > --- a/drivers/media/pci/intel/ipu-bridge.c > +++ b/drivers/media/pci/intel/ipu-bridge.c > @@ -220,7 +220,6 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge, > struct ipu_sensor *sensor) > { > struct software_node *nodes = sensor->swnodes; > - char vcm_name[ACPI_ID_LEN + 4]; > > ipu_bridge_init_swnode_names(sensor); > > @@ -240,10 +239,10 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge, > sensor->ipu_properties); > if (sensor->ssdb.vcmtype) { > /* append ssdb.link to distinguish VCM nodes with same HID */ > - snprintf(vcm_name, sizeof(vcm_name), "%s-%u", > - ipu_vcm_types[sensor->ssdb.vcmtype - 1], > + snprintf(sensor->node_names.vcm, sizeof(sensor->node_names.vcm), > + "%s-%u", ipu_vcm_types[sensor->ssdb.vcmtype - 1], > sensor->ssdb.link); > - nodes[SWNODE_VCM] = NODE_VCM(vcm_name); > + nodes[SWNODE_VCM] = NODE_VCM(sensor->node_names.vcm); > } > > ipu_bridge_init_swnode_group(sensor); > diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h > index 8cb733c03e2f..6cce712a0f34 100644 > --- a/drivers/media/pci/intel/ipu-bridge.h > +++ b/drivers/media/pci/intel/ipu-bridge.h > @@ -103,6 +103,7 @@ struct ipu_node_names { > char port[7]; > char endpoint[11]; > char remote_port[7]; > + char vcm[16]; > }; > > struct ipu_sensor_config {
diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c index f0927f80184d..ef6c6cb7b51b 100644 --- a/drivers/media/pci/intel/ipu-bridge.c +++ b/drivers/media/pci/intel/ipu-bridge.c @@ -220,7 +220,6 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge, struct ipu_sensor *sensor) { struct software_node *nodes = sensor->swnodes; - char vcm_name[ACPI_ID_LEN + 4]; ipu_bridge_init_swnode_names(sensor); @@ -240,10 +239,10 @@ static void ipu_bridge_create_connection_swnodes(struct ipu_bridge *bridge, sensor->ipu_properties); if (sensor->ssdb.vcmtype) { /* append ssdb.link to distinguish VCM nodes with same HID */ - snprintf(vcm_name, sizeof(vcm_name), "%s-%u", - ipu_vcm_types[sensor->ssdb.vcmtype - 1], + snprintf(sensor->node_names.vcm, sizeof(sensor->node_names.vcm), + "%s-%u", ipu_vcm_types[sensor->ssdb.vcmtype - 1], sensor->ssdb.link); - nodes[SWNODE_VCM] = NODE_VCM(vcm_name); + nodes[SWNODE_VCM] = NODE_VCM(sensor->node_names.vcm); } ipu_bridge_init_swnode_group(sensor); diff --git a/drivers/media/pci/intel/ipu-bridge.h b/drivers/media/pci/intel/ipu-bridge.h index 8cb733c03e2f..6cce712a0f34 100644 --- a/drivers/media/pci/intel/ipu-bridge.h +++ b/drivers/media/pci/intel/ipu-bridge.h @@ -103,6 +103,7 @@ struct ipu_node_names { char port[7]; char endpoint[11]; char remote_port[7]; + char vcm[16]; }; struct ipu_sensor_config {
Commit 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs with same HID") introduced an on stack vcm_name and then uses this for the name field of the software_node struct used for the vcm. But the software_node struct is much longer lived then the current stack-frame, so this is no good. Instead extend the ipu_node_names struct with an extra field to store the vcm software_node name and use that. Note this also changes the length of the allocated buffer from ACPI_ID_LEN + 4 to 16. the name is filled with "<ipu_vcm_types[x]>-%u" where ipu_vcm_types[x] is not an ACPI_ID. The maximum length of the strings in the ipu_vcm_types[] array is 11 + 5 bytes for "-255\0" means 16 bytes are needed in the worst case scenario. Fixes: 567f97bd381f ("media: ipu3-cio2: support multiple sensors and VCMs with same HID") Cc: Bingbu Cao <bingbu.cao@intel.com> Reviewed-by: Andy Shevchenko <andy@kernel.org> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/pci/intel/ipu-bridge.c | 7 +++---- drivers/media/pci/intel/ipu-bridge.h | 1 + 2 files changed, 4 insertions(+), 4 deletions(-)