[PATCH v5 1/9] rust: pwm: Add Kconfig and basic data structures
Uwe Kleine-König
ukleinek at kernel.org
Tue Jul 1 06:47:51 PDT 2025
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20250701/5e6b8a15/attachment.sig>
More information about the linux-riscv
mailing list