Re: [PATCH v4 04/11] driver core: Constify API device_find_child() then adapt for various usages
On Wed, Dec 11, 2024 at 08:08:06AM +0800, Zijun Hu wrote:
> From: Zijun Hu
>
> Constify the following API:
> struct device *device_find_child(struct device *dev, void *data,
> int (*match)(struct device *dev, void *data));
> To :
> struct device *device_find_child(struct device *dev, const void *data,
> device_match_t match);
> typedef int (*device_match_t)(struct device *dev, const void *data);
> with the following reasons:
>
> - Protect caller's match data @*data which is for comparison and lookup
> and the API does not actually need to modify @*data.
>
> - Make the API's parameters (@match)() and @data have the same type as
> all of other device finding APIs (bus|class|driver)_find_device().
>
> - All kinds of existing device match functions can be directly taken
> as the API's argument, they were exported by driver core.
>
> Constify the API and adapt for various existing usages by simply making
> various match functions take 'const void *' as type of match data @data.
>
> Reviewed-by: Alison Schofield
> Reviewed-by: Takashi Sakamoto
> Signed-off-by: Zijun Hu
> ---
> arch/sparc/kernel/vio.c| 6 +++---
> drivers/base/core.c| 6 +++---
> drivers/block/sunvdc.c | 6 +++---
> drivers/bus/fsl-mc/dprc-driver.c | 4 ++--
> drivers/cxl/core/pci.c | 4 ++--
> drivers/cxl/core/pmem.c| 2 +-
> drivers/cxl/core/region.c | 21 -
> drivers/firewire/core-device.c | 4 ++--
> drivers/firmware/arm_scmi/bus.c| 4 ++--
> drivers/firmware/efi/dev-path-parser.c | 4 ++--
> drivers/gpio/gpio-sim.c| 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +-
> drivers/hwmon/hwmon.c | 2 +-
> drivers/media/pci/mgb4/mgb4_core.c | 4 ++--
> drivers/nvdimm/bus.c | 2 +-
> drivers/pwm/core.c | 2 +-
> drivers/rpmsg/rpmsg_core.c | 4 ++--
Reviewed-by: Mathieu Poirier
> drivers/scsi/qla4xxx/ql4_os.c | 3 ++-
> drivers/scsi/scsi_transport_iscsi.c| 10 +-
> drivers/slimbus/core.c | 8
> drivers/thunderbolt/retimer.c | 2 +-
> drivers/thunderbolt/xdomain.c | 2 +-
> drivers/tty/serial/serial_core.c | 4 ++--
> drivers/usb/typec/class.c | 8
> include/linux/device.h | 4 ++--
> include/scsi/scsi_transport_iscsi.h| 4 ++--
> net/dsa/dsa.c | 2 +-
> tools/testing/cxl/test/cxl.c | 2 +-
> 28 files changed, 66 insertions(+), 62 deletions(-)
>
> diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
> index
> 07933d75ac815160a2580dce39fde7653a9502e1..1a1a9d6b8f2e8dfedefafde846315a06a167fbfb
> 100644
> --- a/arch/sparc/kernel/vio.c
> +++ b/arch/sparc/kernel/vio.c
> @@ -419,13 +419,13 @@ struct vio_remove_node_data {
> u64 node;
> };
>
> -static int vio_md_node_match(struct device *dev, void *arg)
> +static int vio_md_node_match(struct device *dev, const void *arg)
> {
> struct vio_dev *vdev = to_vio_dev(dev);
> - struct vio_remove_node_data *node_data;
> + const struct vio_remove_node_data *node_data;
> u64 node;
>
> - node_data = (struct vio_remove_node_data *)arg;
> + node_data = (const struct vio_remove_node_data *)arg;
>
> node = vio_vdev_node(node_data->hp, vdev);
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index
> 94865c9d8adcf5f2ce5002ffd7bf0ef4fc85e4d7..bc3b523a4a6366080c3c9fd190e54c7fd13c8ded
> 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4079,8 +4079,8 @@ EXPORT_SYMBOL_GPL(device_for_each_child_reverse_from);
> *
> * NOTE: you will need to drop the reference with put_device() after use.
> */
> -struct device *device_find_child(struct device *parent, void *data,
> - int (*match)(struct device *dev, void *data))
> +struct device *device_find_child(struct device *parent, const void *data,
> + device_match_t match)
> {
> struct klist_iter i;
> struct device *child;
> @@ -4125,7 +4125,7 @@ struct device *device_find_child_by_name(struct device
> *parent,
> }
> EXPORT_SYMBOL_GPL(device_find_child_by_name);
>
> -static int match_any(struct device *dev, void *unused)
> +static int match_any(struct device *dev, const void *unused)
> {
> return 1;
> }
> diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c
> index
> 2d38331ee66793402e803ec0cc82e9e71c991c84..386643ceed59921203828844aa070833c44c67fb
> 100644
> --- a/drivers/block/sunvdc.c
> +++ b/drivers/block/sunvdc.c
> @@ -918,12 +918,12 @@ struct vdc_check_port_data {
> char*type;
> };
>
> -static int vdc_device_probed(struct device *dev, void *arg)
> +static int vdc_device_probed(struct device *dev, const void *arg)
> {
>
Re: [PATCH v4 04/11] driver core: Constify API device_find_child() then adapt for various usages
On 2024/12/24 04:33, Jonathan Cameron wrote:
> On Wed, 11 Dec 2024 08:08:06 +0800
> Zijun Hu wrote:
>
>> From: Zijun Hu
>>
>> Constify the following API:
>> struct device *device_find_child(struct device *dev, void *data,
>> int (*match)(struct device *dev, void *data));
>> To :
>> struct device *device_find_child(struct device *dev, const void *data,
>> device_match_t match);
>> typedef int (*device_match_t)(struct device *dev, const void *data);
>> with the following reasons:
>>
>> - Protect caller's match data @*data which is for comparison and lookup
>> and the API does not actually need to modify @*data.
>>
>> - Make the API's parameters (@match)() and @data have the same type as
>> all of other device finding APIs (bus|class|driver)_find_device().
>>
>> - All kinds of existing device match functions can be directly taken
>> as the API's argument, they were exported by driver core.
>>
>> Constify the API and adapt for various existing usages by simply making
>> various match functions take 'const void *' as type of match data @data.
>
> There are a couple of places I noticed where you changed types
> that aren't related to the specific change this is making.
> They are almost certainly fine but I'd either like a specific note
> on that in this patch description or just change the elements
> that are directly affected by this change.
>
okay, will correct commit message in v5.
v5 will mention these changes will bring extra code improvement as side
effects during achieving main purpose.
>>
>> Reviewed-by: Alison Schofield
>> Reviewed-by: Takashi Sakamoto
>> Signed-off-by: Zijun Hu
>
>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index
>> d778996507984a759bbe84e7acac3774e0c7af98..bfecd71040c2f4373645380b4c31327d8b42d095
>> 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
>
>> @@ -1722,10 +1722,12 @@ static struct cxl_port *next_port(struct cxl_port
>> *port)
>> return port->parent_dport->port;
>> }
>>
>> -static int match_switch_decoder_by_range(struct device *dev, void *data)
>> +static int match_switch_decoder_by_range(struct device *dev,
>> + const void *data)
>> {
>> struct cxl_switch_decoder *cxlsd;
>> -struct range *r1, *r2 = data;
>> +const struct range *r1, *r2 = data;
>
> As below. I'd not touch type of things you don't need to touch.
>
explained below.
>> +
>>
>> if (!is_switch_decoder(dev))
>> return 0;
>> @@ -3176,9 +3178,10 @@ static int devm_cxl_add_dax_region(struct cxl_region
>> *cxlr)
>> return rc;
>> }
>>
>> -static int match_root_decoder_by_range(struct device *dev, void *data)
>> +static int match_root_decoder_by_range(struct device *dev,
>> + const void *data)
>> {
>> -struct range *r1, *r2 = data;
>> +const struct range *r1, *r2 = data;
>
> From the point of view of keeping the patch to what it 'needs'
> to touch, should leave type of r1 alone.
> I've not looked at whether this causes any problems, just whether
> it is relevant to this change.
>
1) i have checked that will not cause problem, may have code improvement
2) this change (2 lines) is simpler than below(3 lines)
-struct range *r1, *r2 = data;
+struct range *r1;
+const struct range *r2 = data;
3) r1 and r2 are used for range_contains() whose prototype was changed
to takes 2 const pointers recently.
4) make r1 & r2 keep the same type as original.
>> struct cxl_root_decoder *cxlrd;
>>
>> if (!is_root_decoder(dev))
>> @@ -3189,11 +3192,11 @@ static int match_root_decoder_by_range(struct device
>> *dev, void *data)
>> return range_contains(r1, r2);
>> }
>>
>> -static int match_region_by_range(struct device *dev, void *data)
>> +static int match_region_by_range(struct device *dev, const void *data)
>> {
>> struct cxl_region_params *p;
>> struct cxl_region *cxlr;
>> -struct range *r = data;
>> +const struct range *r = data;
>> int rc = 0;
>>
>
Re: [PATCH v4 04/11] driver core: Constify API device_find_child() then adapt for various usages
On Wed, 11 Dec 2024 08:08:06 +0800
Zijun Hu wrote:
> From: Zijun Hu
>
> Constify the following API:
> struct device *device_find_child(struct device *dev, void *data,
> int (*match)(struct device *dev, void *data));
> To :
> struct device *device_find_child(struct device *dev, const void *data,
> device_match_t match);
> typedef int (*device_match_t)(struct device *dev, const void *data);
> with the following reasons:
>
> - Protect caller's match data @*data which is for comparison and lookup
> and the API does not actually need to modify @*data.
>
> - Make the API's parameters (@match)() and @data have the same type as
> all of other device finding APIs (bus|class|driver)_find_device().
>
> - All kinds of existing device match functions can be directly taken
> as the API's argument, they were exported by driver core.
>
> Constify the API and adapt for various existing usages by simply making
> various match functions take 'const void *' as type of match data @data.
There are a couple of places I noticed where you changed types
that aren't related to the specific change this is making.
They are almost certainly fine but I'd either like a specific note
on that in this patch description or just change the elements
that are directly affected by this change.
>
> Reviewed-by: Alison Schofield
> Reviewed-by: Takashi Sakamoto
> Signed-off-by: Zijun Hu
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index
> d778996507984a759bbe84e7acac3774e0c7af98..bfecd71040c2f4373645380b4c31327d8b42d095
> 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1722,10 +1722,12 @@ static struct cxl_port *next_port(struct cxl_port
> *port)
> return port->parent_dport->port;
> }
>
> -static int match_switch_decoder_by_range(struct device *dev, void *data)
> +static int match_switch_decoder_by_range(struct device *dev,
> + const void *data)
> {
> struct cxl_switch_decoder *cxlsd;
> - struct range *r1, *r2 = data;
> + const struct range *r1, *r2 = data;
As below. I'd not touch type of things you don't need to touch.
> +
>
> if (!is_switch_decoder(dev))
> return 0;
> @@ -3176,9 +3178,10 @@ static int devm_cxl_add_dax_region(struct cxl_region
> *cxlr)
> return rc;
> }
>
> -static int match_root_decoder_by_range(struct device *dev, void *data)
> +static int match_root_decoder_by_range(struct device *dev,
> +const void *data)
> {
> - struct range *r1, *r2 = data;
> + const struct range *r1, *r2 = data;
From the point of view of keeping the patch to what it 'needs'
to touch, should leave type of r1 alone.
I've not looked at whether this causes any problems, just whether
it is relevant to this change.
> struct cxl_root_decoder *cxlrd;
>
> if (!is_root_decoder(dev))
> @@ -3189,11 +3192,11 @@ static int match_root_decoder_by_range(struct device
> *dev, void *data)
> return range_contains(r1, r2);
> }
>
> -static int match_region_by_range(struct device *dev, void *data)
> +static int match_region_by_range(struct device *dev, const void *data)
> {
> struct cxl_region_params *p;
> struct cxl_region *cxlr;
> - struct range *r = data;
> + const struct range *r = data;
> int rc = 0;
>
Re: [PATCH v4 04/11] driver core: Constify API device_find_child() then adapt for various usages
Hello, On Wed, Dec 11, 2024 at 08:08:06AM +0800, Zijun Hu wrote: > drivers/pwm/core.c | 2 +- Acked-by: Uwe Kleine-König # for drivers/pwm Best regards Uwe signature.asc Description: PGP signature
