Messages in this thread | | | Date | Sun, 7 Aug 2022 13:23:32 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH v1 4/4] pwm: sysfs: Utilize an array for polarity strings |
| |
Hello Andy,
On Sun, Aug 07, 2022 at 12:23:31AM +0300, Andy Shevchenko wrote: > Code is smaller and looks nicer if we combine polarity strings into an array. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pwm/sysfs.c | 32 ++++++++++++-------------------- > 1 file changed, 12 insertions(+), 20 deletions(-) > > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c > index 767c4b19afb1..1bbc5286b7c6 100644 > --- a/drivers/pwm/sysfs.c > +++ b/drivers/pwm/sysfs.c > @@ -151,27 +151,23 @@ static ssize_t enable_store(struct device *child, > return ret ? : size; > } > > +static const char * const polarity_strings[] = {
I like having function and variable prefixes, so I'd prefer this to be called pwm_polarity_strings[]. (Side note: The device show and store callbacks obviously don't have a prefix either because DEVICE_ATTR_RW et al force the functions to be called "${name}_show" and "${name}_store". I considered already a few times to introduce something like
#define __ATTR_NS_RW_MODE(_name, _ns,_mode) { \ .attr = { .name = __stringify(_name), \ .mode = VERIFY_OCTAL_PERMISSIONS(_mode) }, \ .show = _ns ## _ ## _name ## _show, \ .store = _ns ## _ ## _name ## _store, \ }
#define DEVICE_ATTR_NS_RW(_name, _ns) \ struct device_attribute _ns ## _dev_attr_ ## _name = __ATTR_NS_RW_MODE(_name, _ns, 0600)
To allow the functions to have a name space. Never came around to do that though.)
> + [PWM_POLARITY_NORMAL] = "normal", > + [PWM_POLARITY_INVERSED] = "inversed",
I slightly prefer to not align the = in such definitions. Using a single plain space before = is already used in the definiton of pwm_class in the same file.
Otherwise the patch looks fine.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |