| 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: <mipf6ogg45h5bsdekr27sf3nfllbbylkqjiowutg5cugbyosy4@r4glajhjcorn>
Date: Mon, 10 Nov 2025 12:17:57 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Biju Das <biju.das.jz@...renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>,
Magnus Damm <magnus.damm@...il.com>, linux-pwm@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
linux-kernel@...r.kernel.org, Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
Biju Das <biju.das.au@...il.com>
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-biju.das.jz@bp.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/20221214132232.2835828-3-biju.das.jz@bp.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.
> + cells >>= 1;
Is it an error if cells is an odd number?
> + 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.
> + }
> +
> + 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.
> + }
> +
> + if (!of_property_read_u32(of_args.np, "renesas,poeg-id", &poeg_grp)) {
> + if (poeg_grp > RZG2L_LAST_POEG_GROUP) {
> + dev_err(&pdev->dev, "Invalid poeg group %d > %d\n",
> + poeg_grp, RZG2L_LAST_POEG_GROUP);
> + of_node_put(of_args.np);
> + return;
> + }
> +
> + bitpos = of_args.args[0] + poeg_grp * RZG2L_MAX_HW_CHANNELS;
> + set_bit(bitpos, rzg2l_gpt->poeg_gpt_link);
> +
> + rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTINTAD(of_args.args[0]),
> + RZG2L_GTINTAD_GRP_MASK,
> + poeg_grp << 24);
> +
> + rzg2l_gpt_modify(rzg2l_gpt, RZG2L_GTIOR(of_args.args[0]),
> + RZG2L_GTIOR_OBDF | RZG2L_GTIOR_OADF,
> + RZG2L_GTIOR_PIN_DISABLE_SETTING);
> + }
> +
> + of_node_put(of_args.np);
> + }
> +}
Best regards
Uwe
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists