Hi Biju,
On Tue, May 10, 2022 at 5:11 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> This patch add support for linking POEG group with pwm, so that
> POEG can control the output disable function.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
Thanks for your patch!
> --- a/drivers/pwm/pwm-rzg2l-gpt.c
> +++ b/drivers/pwm/pwm-rzg2l-gpt.c
> @@ -266,6 +291,36 @@ static int rzg2l_gpt_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> return ret;
> }
>
> +static int rzg2l_gpt_parse_properties(struct platform_device *pdev,
> + struct rzg2l_gpt_chip *pc)
> +{
> + static const u64 poeg_grp_addr[] = {
> + POEG_GRP_A_ADDR, POEG_GRP_B_ADDR, POEG_GRP_C_ADDR, POEG_GRP_D_ADDR
> + };
> + struct device_node *np;
> + unsigned int i;
> + u64 addr;
> +
> + pc->poeg_grp = GRP_INVALID;
> + np = of_parse_phandle(pdev->dev.of_node, "renesas,poeg-group", 0);
> + if (!np)
> + return 0;
> +
> + if (!of_property_read_u64(np, "reg", &addr)) {
> + for (i = 0; i < ARRAY_SIZE(poeg_grp_addr); i++) {
> + if (addr == poeg_grp_addr[i]) {
Matching on addresses looks fragile to me.
Of course this is code, not DT, so it can be changed later.
Possible alternatives:
1. Use a numeric property instead of a phandle, so you can store
its value directly into pc->poeg_grp.
This loses the linking by phandle, though, which is nice to
have, and might be useful for other purposes later.
2. Add a "renesas,id" property to each POEGx DT node, cfr.
Documentation/devicetree/bindings/media/renesas,vin.yaml.
> + pc->poeg_grp = i;
> + break;
> + }
> + }
> + }
> +
> + if (np)
> + of_node_put(np);
> +
> + return 0;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
