Hello, On Thu, Dec 15, 2022 at 08:58:43PM +0000, Biju Das wrote: > The General PWM Timer (GPT) is capable of detecting "dead time error > and short-circuits between output pins" and send Output disable > request to poeg(Port Output Enable for GPT). > > This patch add support for linking poeg group with gpt, so that > gpt can control the output disable function. > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > --- > v2->v3: > * Updated commit header and description > * Added check for poeg group in rzg2l_gpt_parse_properties(). > v1->v2: > * Replaced id->poeg-id as per poeg bindings. > This patch depend upon [1] > [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20221214132232.2835828-3-biju.das.jz@xxxxxxxxxxxxxx/ > --- > drivers/pwm/pwm-rzg2l-gpt.c | 76 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c > index fa916f020061..6bf407550326 100644 > --- a/drivers/pwm/pwm-rzg2l-gpt.c > +++ b/drivers/pwm/pwm-rzg2l-gpt.c > @@ -31,6 +31,7 @@ > #define RZG2L_GTCR 0x2c > #define RZG2L_GTUDDTYC 0x30 > #define RZG2L_GTIOR 0x34 > +#define RZG2L_GTINTAD 0x38 > #define RZG2L_GTBER 0x40 > #define RZG2L_GTCNT 0x48 > #define RZG2L_GTCCRA 0x4c > @@ -48,9 +49,15 @@ > #define RZG2L_UP_COUNTING (RZG2L_GTUDDTYC_UP | RZG2L_GTUDDTYC_UDF) > > #define RZG2L_GTIOR_GTIOA GENMASK(4, 0) > +#define RZG2L_GTIOR_OADF GENMASK(10, 9) > #define RZG2L_GTIOR_GTIOB GENMASK(20, 16) > +#define RZG2L_GTIOR_OBDF GENMASK(26, 25) > #define RZG2L_GTIOR_OAE BIT(8) > #define RZG2L_GTIOR_OBE BIT(24) > +#define RZG2L_GTIOR_OADF_HIGH_IMP_ON_OUT_DISABLE BIT(9) > +#define RZG2L_GTIOR_OBDF_HIGH_IMP_ON_OUT_DISABLE BIT(25) > +#define RZG2L_GTIOR_PIN_DISABLE_SETTING \ > + (RZG2L_GTIOR_OADF_HIGH_IMP_ON_OUT_DISABLE | RZG2L_GTIOR_OBDF_HIGH_IMP_ON_OUT_DISABLE) > > #define RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE 0x07 > #define RZG2L_INIT_OUT_HI_OUT_HI_END_TOGGLE 0x1b > @@ -64,12 +71,16 @@ > #define RZG2L_GTIOR_GTIOB_OUT_LO_END_TOGGLE_CMP_MATCH \ > (FIELD_PREP(RZG2L_GTIOR_GTIOB, RZG2L_INIT_OUT_LO_OUT_LO_END_TOGGLE) | RZG2L_GTIOR_OBE) > > +#define RZG2L_GTINTAD_GRP_MASK GENMASK(25, 24) > + > #define RZG2L_GTCCR(i) (0x4c + 4 * (i)) > > #define RZG2L_MAX_HW_CHANNELS (8) > #define RZG2L_CHANNELS_PER_IO (2) > #define RZG2L_MAX_PWM_CHANNELS (RZG2L_MAX_HW_CHANNELS * RZG2L_CHANNELS_PER_IO) > > +#define RZG2L_MAX_POEG_GROUPS (4) The parenthesis are not needed (ditto for RZG2L_MAX_HW_CHANNELS and RZG2L_CHANNELS_PER_IO). > + > #define RZG2L_IS_IOB(a) ((a) & 0x1) > #define RZG2L_GET_CH_INDEX(a) ((a) / 2) > > @@ -91,6 +102,7 @@ struct rzg2l_gpt_chip { > */ > u8 prescale[RZG2L_MAX_HW_CHANNELS]; > unsigned int duty_cycle[RZG2L_MAX_PWM_CHANNELS]; > + DECLARE_BITMAP(poeg_gpt_link, RZG2L_MAX_POEG_GROUPS * RZG2L_MAX_HW_CHANNELS); > }; > > static inline struct rzg2l_gpt_chip *to_rzg2l_gpt_chip(struct pwm_chip *chip) > @@ -470,6 +482,69 @@ static void rzg2l_gpt_reset_assert_pm_disable(void *data) > reset_control_assert(rzg2l_gpt->rstc); > } > A comment here about the purpose of the function would be nice. Just from reading the code it's totally unobvious what happens here. > +static void rzg2l_gpt_parse_properties(struct platform_device *pdev, > + struct rzg2l_gpt_chip *rzg2l_gpt) > +{ > + struct of_phandle_args of_args; > + unsigned int i; > + u32 poeg_grp; > + u32 bitpos; > + int cells; > + u32 offs; > + int ret; > + > + cells = of_property_count_u32_elems(pdev->dev.of_node, "renesas,poegs"); > + if (cells == -EINVAL) > + return; > + > + cells >>= 1; > + for (i = 0; i < cells; i++) { > + ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node, > + "renesas,poegs", 1, i, > + &of_args); > + if (ret) { > + dev_err(&pdev->dev, > + "Failed to parse 'renesas,poegs' property\n"); > + return; > + } > + > + if (of_args.args[0] >= RZG2L_MAX_HW_CHANNELS) { > + dev_err(&pdev->dev, > + "Invalid channel %d > 7\n", of_args.args[0]); this hardcoded 7 is a bit ugly. Something like + dev_err(&pdev->dev, + "Invalid channel %d >= %d\n", of_args.args[0], RZG2L_MAX_HW_CHANNELS); or + dev_err(&pdev->dev, + "Invalid channel %d >= " stringify(RZG2L_MAX_HW_CHANNELS) "\n", of_args.args[0]); is IMHO nicer. > + return; > + } > + > + bitpos = of_args.args[0]; > + if (!of_device_is_available(of_args.np)) { > + /* It's fine to have a phandle to a non-enabled poeg. */ > + of_node_put(of_args.np); > + continue; > + } > + > + if (!of_property_read_u32(of_args.np, "renesas,poeg-id", &poeg_grp)) { > + offs = RZG2L_GET_CH_OFFS(of_args.args[0]); > + if (poeg_grp > 3) { Maybe a cpp define for this 3? > + dev_err(&pdev->dev, > + "Invalid poeg group %d > 3\n", poeg_grp); You're missing + of_node_put(of_args.np); here. > + return; > + } > + > + bitpos += poeg_grp * RZG2L_MAX_HW_CHANNELS; > + set_bit(bitpos, rzg2l_gpt->poeg_gpt_link); > + > + rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTINTAD, > + RZG2L_GTINTAD_GRP_MASK, > + poeg_grp << 24); > + > + rzg2l_gpt_modify(rzg2l_gpt, offs + RZG2L_GTIOR, > + RZG2L_GTIOR_OBDF | RZG2L_GTIOR_OADF, > + RZG2L_GTIOR_PIN_DISABLE_SETTING); > + } > + > + of_node_put(of_args.np); > + } > +} > + > static int rzg2l_gpt_probe(struct platform_device *pdev) > { > DECLARE_BITMAP(ch_en_bits, RZG2L_MAX_PWM_CHANNELS); > @@ -512,6 +587,7 @@ static int rzg2l_gpt_probe(struct platform_device *pdev) > if (ret < 0) > goto clk_disable; > > + rzg2l_gpt_parse_properties(pdev, rzg2l_gpt); I don't like the function name. THe function doesn't only parse the properties but also implements the needed register writes. Maybe rzg2l_gpt_poeg_init()? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
