> -----Original Message----- > From: linux-pwm-owner@xxxxxxxxxxxxxxx <linux-pwm-owner@xxxxxxxxxxxxxxx> > On Behalf Of Uwe Kleine-König > Sent: Sunday, May 24, 2020 5:05 AM > To: G Jaya Kumaran, Vineetha <vineetha.g.jaya.kumaran@xxxxxxxxx> > Cc: thierry.reding@xxxxxxxxx; robh+dt@xxxxxxxxxx; linux- > pwm@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Wan Mohamad, Wan > Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@xxxxxxxxx>; Shevchenko, > Andriy <andriy.shevchenko@xxxxxxxxx> > Subject: Re: [PATCH 1/3] pwm: Add count attribute in sysfs for Intel Keem Bay > > On Sun, May 17, 2020 at 09:52:38PM +0800, > vineetha.g.jaya.kumaran@xxxxxxxxx wrote: > > From: "Lai, Poey Seng" <poey.seng.lai@xxxxxxxxx> > > > > In Keem Bay, the number of repetitions for the period/waveform can be > > configured from userspace. This requires addition of a sysfs attribute > > to get/set the repetition count. Setting this value to 0 will result > > in continuous repetition of the waveform until the channel is disabled > > or reconfigured. > > There is nothing specific for Keem Bay in this patch, the only special thing here is > that this driver is the first implementor. > > IMHO all drivers that don't support count should be changed to fail if a count > 0 > is passed and introducing support in the sysfs interface should be a separate > change. > > Having said that I'm not convinced this is a good idea given that only very few > driver can support this interface. Also this needs documentation, e.g. what is > expected from .get_state(). > > You should also motivate what this functionality is used for in the commit log > and I'd prefer to see an in-tree user (apart from sysfs which I don't count as > such). > Agreed, the wording used here is not accurate as this is not specific for Keem Bay. Before submitting v2, I will cross-check about the use-case for this functionality, and if it can be implemented in some other less intrusive way to the framework (perhaps via a DT property?) > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | https://www.pengutronix.de/ |
