Re: [PATCH v1 3/3] pinctrl: th1520: Factor out casts

Drew Fustini
2024-10-11T16:35:27Z

On Fri, Oct 11, 2024 at 04:48:25PM +0200, Emil Renner Berthing wrote:
> Limit the casts to get the mux data and flags from the driver data
> pointer with each pin to two inline functions as requested by Andy
> during review.
>
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index 8bd40cb2f013..7474d8da32f9 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -152,6 +152,16 @@ static enum th1520_muxtype th1520_muxtype_get(const char *str)
> (TH1520_MUX_##m0 << 0) | (TH1520_MUX_##m1 << 5) | (TH1520_MUX_##m2 << 10) | \
> (TH1520_MUX_##m3 << 15) | (TH1520_MUX_##m4 << 20) | (TH1520_MUX_##m5 << 25)) }
>
> +static unsigned long th1520_pad_muxdata(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_MUXDATA;
> +}
> +
> +static bool th1520_pad_no_padcfg(void *drv_data)
> +{
> + return (uintptr_t)drv_data & TH1520_PAD_NO_PADCFG;
> +}
> +
> static const struct pinctrl_pin_desc th1520_group1_pins[] = {
> TH1520_PAD(0, OSC_CLK_IN, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> TH1520_PAD(1, OSC_CLK_OUT, ____, ____, ____, ____, ____, ____, TH1520_PAD_NO_PADCFG),
> @@ -590,7 +600,7 @@ static int th1520_pinconf_get(struct pinctrl_dev *pctldev,
> u32 value;
> u32 arg;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> value = readl_relaxed(th1520_padcfg(thp, pin));
> @@ -660,7 +670,7 @@ static int th1520_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> unsigned int i;
> u16 mask, value;
>
> - if ((uintptr_t)desc->drv_data & TH1520_PAD_NO_PADCFG)
> + if (th1520_pad_no_padcfg(desc->drv_data))
> return -ENOTSUPP;
>
> mask = 0;
> @@ -793,12 +803,14 @@ static int th1520_pinmux_set_mux(struct pinctrl_dev *pctldev,
> {
> struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
> const struct function_desc *func = pinmux_generic_get_function(pctldev, fsel);
> + enum th1520_muxtype muxtype = (uintptr_t)func->data;
>
> if (!func)
> return -EINVAL;
> +
> return th1520_pinmux_set(thp, thp->desc.pins[gsel].number,
> - (uintptr_t)thp->desc.pins[gsel].drv_data & TH1520_PAD_MUXDATA,
> - (uintptr_t)func->data);
> + th1520_pad_muxdata(thp->desc.pins[gsel].drv_data),
> + muxtype);
> }
>
> static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> @@ -809,7 +821,7 @@ static int th1520_gpio_request_enable(struct pinctrl_dev *pctldev,
> const struct pin_desc *desc = pin_desc_get(pctldev, offset);
>
> return th1520_pinmux_set(thp, offset,
> - (uintptr_t)desc->drv_data & TH1520_PAD_MUXDATA,
> + th1520_pad_muxdata(desc->drv_data),
> TH1520_MUX_GPIO);
> }
>
> --
> 2.43.0
>

Reviewed-by: Drew Fustini <dfustini@tenstorrent.com>

Thanks for improving this. I see the feedback from Andy [1] on your v2
now that you mention it.

-Drew

[1] https://lore.kernel.org/linux-gpio/Zj8K_0zpI_IAY66R@surfacebook.localdomain/