Re: [PATCH v4 04/11] driver core: Constify API device_find_child() then adapt for various usages

[Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>]

On Wed, 11 Dec 2024 08:08:06 +0800
Zijun Hu <zijun_hu@xxxxxxxxxx> wrote:

> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
> 
> 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 <alison.schofield@xxxxxxxxx>
> Reviewed-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx>
> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>


> 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;