Re: [PATCH v5 1/9] rust: pwm: Add Kconfig and basic data structures

[Uwe Kleine-König]

Hello Michal,

On Tue, Jul 01, 2025 at 10:24:54AM +0200, Michal Wilczynski wrote:
> On 6/29/25 12:29, Uwe Kleine-König wrote:
> > On Sat, Jun 28, 2025 at 09:47:19PM +0200, Michal Wilczynski wrote:
> 
> >>>>> +    /// Sets the polarity of the PWM signal.
> >>>>> +    pub fn set_polarity(&mut self, polarity: Polarity) {
> >>>>> +        self.0.polarity = polarity.into();
> >>>>> +    }
> >>>>
> >>>> Please don't expose these non-atomic callbacks. pwm_disable() would be
> >>>> fine.
> >>
> >> Hmm, I've just realized that without those setters it would most likely
> >> impossible to correctly implement the get_state callback.
> > 
> > You shouldn't implement the get_state callback for a waveform driver.
> 
> You're right that a new driver using the waveform API shouldn't
> implement .get_state.
> 
> My goal for the abstraction layer, however, is to be flexible enough to
> support writing both modern waveform drivers and legacy style drivers
> that use the .apply and .get_state callbacks.

No, please don't. New C drivers should implement the waveform API. The
same holds true for Rust drivers. So don't create a door that isn't
supposed to be used.

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature