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;
