Messages in this thread | | | Date | Wed, 29 Nov 2023 16:14:58 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v5 2/2] iio: pressure: driver for Honeywell HSC/SSC series pressure sensors |
| |
On Wed, Nov 29, 2023 at 03:51:54PM +0200, Petre Rodan wrote: > On Wed, Nov 29, 2023 at 12:36:46PM +0200, Andy Shevchenko wrote: > > On Wed, Nov 29, 2023 at 09:04:49AM +0200, Petre Rodan wrote:
...
> > > + for (index = 0; index < ARRAY_SIZE(hsc_range_config); index++) { > > > + if (strncmp(hsc_range_config[index].triplet, > > > + triplet, > > > + HSC_PRESSURE_TRIPLET_LEN - 1) == 0) { > > > + hsc->pmin = hsc_range_config[index].pmin; > > > + hsc->pmax = hsc_range_config[index].pmax; > > > + found = 1; > > > + break; > > > + } > > > + } > > > + if (hsc->pmin == hsc->pmax || !found) > > > + return dev_err_probe(dev, -EINVAL, > > > + "honeywell,pressure-triplet is invalid\n"); > > > > This one is important. I think I told already twice that this is NIH > > device_property_match_property_string(). Please, use this API directly. > > let me see if we are on the same frequency. > > you told me to use match_string/sysfs_match_string. > using any of those assume having the strings in an array, which means I have to > clutter the code with 242 extra lines in order to split up hsc_range_config: > > enum hsc_variants { > HSC001BA = 0, > HSC1_6BA = 1, > [ .. 115 lines skipped ] > HSC150PG = 117,
HSC_VARIANTS_MAX
> } > > static const char * const hsc_triplet_variants[] = {
static const char * const hsc_triplet_variants[HSC_VARIANTS_MAX] = {
> [HSC001BA] = "001BA", > [HSC1_6BA] = "1.6BA", > [ .. 115 lines skipped ] > [HSC150PG] = "150PG", > }; > > and change my structure to > > static const struct hsc_range_config hsc_range_config[] = {
static const struct hsc_range_config hsc_range_config[HSC_VARIANTS_MAX] = {
> [HSC001BA] = { .pmin = 0, .pmax = 100000 }, > [ .. 117 lines skipped ] > }; > > just to have the privilege of using match_string() on hsc_triplet_variants. > now, which is worse for maintenance? a handful of lines of code that do a > loop of strncmp or the clutter depicted above? > > I can go either way, but I want to make sure you see where this leads.
That's the suggestion, yes.
...
> > > +#ifndef _HSC030PA_H > > > +#define _HSC030PA_H > > > > > +#include <linux/mutex.h> > > > > > +#include <linux/property.h> > > > > Is not used here. > > ok. > since we are here, can you please tell me how is mutex.h used outside of > hsc030pa.c where it was included previously?
I'm not sure I understand you. Maybe I was unclear, sorry for that. I meant the property.h is not used in the _header_ file. Am I mistaken?
-- With Best Regards, Andy Shevchenko
| |