Re: [PATCH v2 1/2] pinctrl: qcom: handle tiles for ACPI boot

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>]

On Wed, Mar 03, 2021 at 10:45:27PM +0800, Shawn Guo wrote:
> On Wed, Mar 03, 2021 at 04:08:09PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 03, 2021 at 09:26:21PM +0800, Shawn Guo wrote:
> > > It's not always the case that DT and ACPI describe hardware resource in
> > > the same schema, even for a single platform.  For example, on SC8180X,
> > > DT uses the tiles schema while ACPI describe memory resource as a single
> > > region.  It patches msm_pinctrl_probe() function to map tiles regions
> > > only for DT.  While for ACPI, it maps the single memory resource and
> > > calculate tile bases with offsets passed from SoC data.
> > 
> > ...
> > 
> > > +#include <linux/acpi.h>
> > 
> > There are at least two possibilities to avoid this:
> 
> So could you explain why we should avoid including this header?

Here you can include it, but it's quite huge in order to have just one little
function out of it. But main point is it seems that relying on firmware type
for the tiles support is fragile.

> >  - use is_of_node(dev_fwnode(dev)), or in case you need board files support,
> >    !(fwnode && is_of_fwnode(fwnode))
> >  - provide the tiles support directly from the driver thru internal data structures
> > 
> >  And to me the second approach seems better, because there is no guarantee that
> >  tiles support is only defined by the fwnode type.

-- 
With Best Regards,
Andy Shevchenko