Re: [PATCH v7 2/2] pwm: sophgo: add pwm support for Sophgo CV1800 SoC

[Inochi Amaoto]

On Tue, Apr 08, 2025 at 09:54:27AM +0200, Thomas Bonnefille wrote:
> On Mon Apr 7, 2025 at 9:21 AM CEST, Inochi Amaoto wrote:
> > On Sun, Apr 06, 2025 at 02:16:41AM +0200, Thomas Bonnefille wrote:
> >> Hello,
> >> 
> >> On Sat Jun 1, 2024 at 1:53 PM CEST, Uwe Kleine-König wrote:
> >> > On Wed, May 01, 2024 at 04:32:42PM +0800, Jingbao Qiu wrote:
> >> >> [...]
> >> >> +	if ((state & BIT(pwm->hwpwm)) && enable)
> >> >> +		regmap_update_bits(priv->map, PWM_CV1800_OE,
> >> >> +				   PWM_CV1800_OE_MASK(pwm->hwpwm),
> >> >> +				   PWM_CV1800_REG_ENABLE(pwm->hwpwm));
> >> >
> >> > This looks strange. If BIT(hwpwm) is already set, set it again?!
> >> > Also if you used the caching implemented in regmap, you don't need to
> >> > make this conditional.
> >> >
> >> 
> >> I was testing the series and noticed indeed an issue in this driver at
> >> those lines. If PWM_CV1800_OE isn't set by something else than the
> >> kernel it will never be set and so, there will never be a PWM outputted.
> >> 
> >> Using :
> >>     if (!(state & BIT(pwm->hwpwm)) && enable)
> >> Solved the issue but as Uwe said you can probably rely on regmap caching
> >> to avoid this condition.
> >> 
> >> >
> >> > ...
> >> > 
> >> 
> >> Do you plan on sending a new iteration some day ? I may have some time
> >> to continue the upstreaming process if you need to.
> >> 
> >> Thank you for this series !
> >> Thomas
> >
> > I suggest checking existing spi-sg2044-nor driver, which may reduce your
> > work for upstreaming.
> >
> > Regards,
> > Inochi
> 
> Hello Inochi,
> 
> Thank you very much, however even after reading it I can't see the link
> between the SPI NOR controller driver of the SG2044 and the PWM driver
> for the CV18XX series ?
> 
> Regards,
> Thomas

I am sorry, I mistake this as the spi-nor one. You may want to check
the SG2044 pwm driver, which should be similiar. You could find it at 
https://lore.kernel.org/all/20250407072056.8629-1-looong.bin@xxxxxxxxx/

Regards,
Inochi