Re: [PATCH v22 2/3] riscv: dts: starfive: Correct pwm nodes

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3261 bytes --]

On Wed, Jul 15, 2026 at 02:25:09AM +0000, Hal Feng wrote:
> > On 26.07.14 11:45, sashiko-bot wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> > to consider:
> > - [Medium] Missing electrical pad configuration properties in `pwm1_pins`
> > after node split.
> > --
> > 
> > --- Patch [2]: [PATCH v22 2/3] riscv: dts: starfive: Correct pwm nodes ---
> > commit af3d5e9905acdead9e2816d09f13f054b179cbde
> > Author: Hal Feng <hal.feng@starfivetech.com>
> > 
> > riscv: dts: starfive: Correct pwm nodes
> > 
> > Each of the StarFive JH7100/JH7110 SoCs has 8 OpenCores PTC IP cores.
> > One OpenCores PTC IP core can output one PWM channel. Change the
> > register size to 0x10, since an OpenCores PTC IP has only 4 32-bit registers.
> > 
> > Fixes: 8d01f741a046 ("riscv: dts: starfive: jh7110: Add PWM node and pins
> > configuration")
> > Fixes: 5e598b99fedf ("riscv: dts: starfive: jh7100: Add PWM node and pins
> > configuration")
> > 
> > > diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > > b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > > index ae1a6aeb0aeaa..85106545090ea 100644
> > > --- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > > +++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
> > 
> > [ ... ]
> > 
> > > -	pwm_pins: pwm-0 {
> > > -		pwm-pins {
> > > +	pwm0_pins: pwm0-0 {
> > > +		pwm0-pins {
> > >  			pinmux = <GPIOMUX(7,
> > >  				  GPO_PWM_PAD_OUT_BIT0,
> > >  				  GPO_PWM_PAD_OE_N_BIT0,
> > > -				  GPI_NONE)>,
> > > -				 <GPIOMUX(5,
> > > +				  GPI_NONE)>;
> > > +			bias-disable;
> > > +			drive-strength = <35>;
> > > +			input-disable;
> > > +			input-schmitt-disable;
> > > +			slew-rate = <0>;
> > > +		};
> > > +	};
> > > +
> > > +	pwm1_pins: pwm1-0 {
> > > +		pwm1-pins {
> > > +			pinmux =  <GPIOMUX(5,
> > >  				  GPO_PWM_PAD_OUT_BIT1,
> > >  				  GPO_PWM_PAD_OE_N_BIT1,
> > >  				  GPI_NONE)>;
> > >  		};
> > >  	};
> > 
> > [Severity: Medium]
> > Should pwm1_pins also include the electrical pad configuration properties?
> > 
> > When the pwm_pins node was split, properties like bias-disable,
> > drive-strength, input-disable, input-schmitt-disable, and slew-rate were
> > configured for pwm0_pins. However, pwm1_pins only has the pinmux
> > configured.
> > 
> > When the kernel applies the pinctrl state for pwm1 during probe, it will only
> > configure the pinmux and skip the pad electrical settings. Does this leave the
> > PWM1 pad in an undefined or default electrical state that could cause weak
> > signal drive or other signal integrity issues depending on the hardware
> > defaults?
> 
> No, I added the new electrical properties for pwm0-pins, so the old electrical properties
> are applied to pwm1-pins. You omitted the original code and got a wrong result. Please
> apply the patches to the mainline before you review them.
> 
> The same reason as I explained before.
> https://lore.kernel.org/all/ZQ2PR01MB1307E58DBE3D3EA2DCA9BE9AE6162@ZQ2PR01MB1307.CHNPR01.prod.partner.outlook.cn/

This is a bot that feeds your patches to an LLM for review.
The bot is not sophisticated enough to check previous revisions for
responses to the same problems.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]