Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Fri, 19 Aug 2022 06:09:55 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     Andy Shevchenko <andy.shevchenko@gmail.com>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Linux Documentation List <linux-doc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-pwm@vger.kernel.org, linux-hwmon@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>,
        Thierry Reding <thierry.reding@gmail.com>,
        Jean Delvare <jdelvare@suse.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v1 1/3] hwmon: (pwm-fan) Make use of device properties
Message-ID: <20220819130955.GB3108215@roeck-us.net>
References: <20220806152517.78159-1-andriy.shevchenko@linux.intel.com>
 <20220818232040.GA3505561@roeck-us.net>
 <CAHp75Vf+-4U6OW4sf+mKsPt8WoRL_jN-sYtzOMzhhH8--+OU1w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAHp75Vf+-4U6OW4sf+mKsPt8WoRL_jN-sYtzOMzhhH8--+OU1w@mail.gmail.com>
Precedence: bulk

On Fri, Aug 19, 2022 at 12:56:42PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 19, 2022 at 2:41 AM Guenter Roeck <linux@roeck-us.net> wrote:
> > On Sat, Aug 06, 2022 at 06:25:15PM +0300, Andy Shevchenko wrote:
> > > Convert the module to be property provider agnostic and allow
> > > it to be used on non-OF platforms.
> > >
> > > Add mod_devicetable.h include.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> >
> > I had another look at this patch. A substantial part of the changes
> > is because device properties don't support of_property_read_u32_index(),
> > reworking the code to use device_property_read_u32_array() instead.
> > Sorry, I don't like it, it results in a substantial number of unnecessary
> > changes. Device properties should support the equivalent of
> > of_property_read_u32_index() instead to simplify conversions.
> 
> Not all (device property) providers can have such API available. Are
> you suggesting to
>  a) alloc memory for entire array;
>  b) cache one for a given index;
>  c) free a memory;
>  d) loop as many times as index op is called.
> 
> Sorry, this is way too far and non-optimal in comparison to the
> substantial number of unnecessary changes (two or three small
> refactorings?).
> 
> Another way is to provide a pwm-fan-acpi, which will be the copy of
> the driver after this patch is applied. I don't think it's a very
> bright idea either.
> 
An alternative might be to split the patch in two parts, one replacing
of_property_read_u32_index() with of_property_read_u32_array() as
preparation, with the above rationale and a note that this is to
prepare for the switch to device properties, and then the actual device
property switch. Some context showing how other conversions handled this
problem would also be nice, though not necessary.

Thanks,
Guenter