| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<TY3PR01MB11346940A5BD83A5AD79FE46486D4A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Thu, 20 Nov 2025 17:00:35 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Uwe Kleine-König <ukleinek@...nel.org>
CC: Geert Uytterhoeven <geert+renesas@...der.be>, magnus.damm
<magnus.damm@...il.com>, "linux-pwm@...r.kernel.org"
<linux-pwm@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@...renesas.com>, biju.das.au
<biju.das.au@...il.com>
Subject: RE: [PATCH v24 4/4] pwm: rzg2l-gpt: Add support for gpt linking with
poeg
Hi Uwe,
Thanks for the feedback.
> -----Original Message-----
> From: Uwe Kleine-König <ukleinek@...nel.org>
> Sent: 10 November 2025 11:18
> Subject: Re: [PATCH v24 4/4] pwm: rzg2l-gpt: Add support for gpt linking with poeg
>
> On Wed, Feb 26, 2025 at 02:45:23PM +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).
> >
> > 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@...renesas.com>
> > ---
> > v23->v24:
> > * No change.
> > v22>v23:
> > * No change
> > v21>v22:
> > * No change
> > v20->21:
> > * Dropped local variable offs for calculating RZG2L_GTINTAD channel register
> > and instead using the macro RZG2L_GTINTAD(ch).
> > v19->v20:
> > * No change
> > v18->v19:
> > * No change
> > v17->v18:
> > * Moved bitpos near to the user.
> > v16->v17:
> > * No change
> > v15->v16:
> > * No change.
> > v14->v15:
> > * Updated commit description by replacing "This patch add"-> "Add".
> > v3->v14:
> > * Removed the parenthesis for RZG2L_MAX_POEG_GROUPS.
> > * Renamed rzg2l_gpt_parse_properties()->rzg2l_gpt_poeg_init() as it not only parse
> > the properties but also implements the needed register writes.
> > * Added acomment here about the purpose of the function
> > rzg2l_gpt_poeg_init()
> > * Removed magic numbers from rzg2l_gpt_poeg_init()
> > * Fixed resource leak in rzg2l_gpt_poeg_init().
> > * Moved the patch from series[1] to here [1]
> > https://lore.kernel.org/linux-renesas-soc/20221215205843.4074504-1-bij
> > u.das.jz@...renesas.com/T/#t
> > 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/202212141
> > 32232.2835828-3-biju.das.jz@...renesas.com/
> > ---
> > drivers/pwm/pwm-rzg2l-gpt.c | 83
> > +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 83 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-rzg2l-gpt.c b/drivers/pwm/pwm-rzg2l-gpt.c
> > index 2ddbb13f50aa..a551554aec77 100644
> > --- a/drivers/pwm/pwm-rzg2l-gpt.c
> > +++ b/drivers/pwm/pwm-rzg2l-gpt.c
> > @@ -39,6 +39,7 @@
> > #define RZG2L_GTCR(ch) (0x2c + RZG2L_GET_CH_OFFS(ch))
> > #define RZG2L_GTUDDTYC(ch) (0x30 + RZG2L_GET_CH_OFFS(ch))
> > #define RZG2L_GTIOR(ch) (0x34 + RZG2L_GET_CH_OFFS(ch))
> > +#define RZG2L_GTINTAD(ch) (0x38 + RZG2L_GET_CH_OFFS(ch))
> > #define RZG2L_GTBER(ch) (0x40 + RZG2L_GET_CH_OFFS(ch))
> > #define RZG2L_GTCNT(ch) (0x48 + RZG2L_GET_CH_OFFS(ch))
> > #define RZG2L_GTCCR(ch, sub_ch) (0x4c + RZG2L_GET_CH_OFFS(ch) + 4 * (sub_ch))
> > @@ -55,12 +56,21 @@
> > #define RZG2L_GTUDDTYC_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_GTIOx(sub_ch) ((sub_ch) ? RZG2L_GTIOR_GTIOB : RZG2L_GTIOR_GTIOA)
> > +
> > #define RZG2L_GTIOR_OAE BIT(8)
> > #define RZG2L_GTIOR_OBE BIT(24)
> > #define RZG2L_GTIOR_OxE(sub_ch) ((sub_ch) ? RZG2L_GTIOR_OBE : RZG2L_GTIOR_OAE)
> >
> > +#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_HI_OUT_HI_END_TOGGLE 0x1b
> > #define RZG2L_GTIOR_GTIOA_OUT_HI_END_TOGGLE_CMP_MATCH \
> > (RZG2L_INIT_OUT_HI_OUT_HI_END_TOGGLE | RZG2L_GTIOR_OAE) @@ -71,12
> > +81,17 @@
> > ((sub_ch) ? RZG2L_GTIOR_GTIOB_OUT_HI_END_TOGGLE_CMP_MATCH : \
> > RZG2L_GTIOR_GTIOA_OUT_HI_END_TOGGLE_CMP_MATCH)
> >
> > +#define RZG2L_GTINTAD_GRP_MASK GENMASK(25, 24)
> > +
> > #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_SCALE_FACTOR 1024
> > #define RZG2L_MAX_TICKS ((u64)U32_MAX * RZG2L_MAX_SCALE_FACTOR)
> >
> > +#define RZG2L_MAX_POEG_GROUPS 4
> > +#define RZG2L_LAST_POEG_GROUP 3
> > +
> > struct rzg2l_gpt_chip {
> > void __iomem *mmio;
> > struct mutex lock; /* lock to protect shared channel resources */ @@
> > -84,6 +99,7 @@ struct rzg2l_gpt_chip {
> > u32 period_ticks[RZG2L_MAX_HW_CHANNELS];
> > u32 channel_request_count[RZG2L_MAX_HW_CHANNELS];
> > u32 channel_enable_count[RZG2L_MAX_HW_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) @@ -362,6 +378,72 @@ static const struct pwm_ops rzg2l_gpt_ops = {
> > .apply = rzg2l_gpt_apply,
> > };
> >
> > +/*
> > + * This function links a poeg group{A,B,C,D} with a gpt channel{0..7}
> > +and
> > + * configure the pin for output disable.
> > + */
> > +static void rzg2l_gpt_poeg_init(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;
> > + int ret;
> > +
> > + cells = of_property_count_u32_elems(pdev->dev.of_node, "renesas,poegs");
> > + if (cells == -EINVAL)
> > + return;
>
> Please catch other errors, too.
Ok.
>
> > + cells >>= 1;
>
> Is it an error if cells is an odd number?
Yes. It expects (POEG phandle)/pwm channel pair
>
> > + 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;
>
> So .probe() might emit an error message now, but it doesn't fail. I would suggest to change the
> latter.
OK, Will do.
>
> > + }
> > +
> > + if (of_args.args[0] >= RZG2L_MAX_HW_CHANNELS) {
> > + dev_err(&pdev->dev, "Invalid channel %d >= %d\n",
> > + of_args.args[0], RZG2L_MAX_HW_CHANNELS);
> > + of_node_put(of_args.np);
> > + return;
> > + }
> > +
> > + 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;
>
> Does of_device_is_available() return false if the poeg is enabled, but not yet probed? In that
> case .probe() should return -EPROBE_DEFER.
No. Returns true if the status property is absent or set to "okay" or "ok
renesas,poegs = <&poeggd 4>;--> this will make poeg probed first.
Cheers,
Biju
Powered by blists - more mailing lists