Re: [PATCH v3 6/9] mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI

[Xu Yilun]

On 2022-12-05 at 11:31:06 +0200, Ilpo Järvinen wrote:
> On Fri, 2 Dec 2022, Russ Weight wrote:
> > On 12/2/22 08:28, Xu Yilun wrote:
> > > On 2022-12-02 at 12:08:38 +0200, Ilpo Järvinen wrote:
> > >> Move SPI based board definitions to per interface file from the global
> > >> header. This makes it harder to use them accidently in the
> > >> generic/interface agnostic code. Prefix the defines with M10BMC_SPI
> > > I'm not sure if the register layout is actually bound to the bus
> > > interface. My experience is the register layout is always decided by
> > > board type. Is it possible there will be a new SPI based board but
> > > has different register layout in future?
> > >
> > > So is M10BMC_SPI_XXX a good nam
> > 
> > There could be future devices, spi or pmci based, that require different
> > addresses for some of these values, and at that time we would need to
> > additional versions of some of these macros using different names.
> > Right now, spi and pmci are the primary differentiating factors. I'm not
> > sure how to improve on the naming. Do you have any suggestions?
> 
> It's per board type yes, but there's a strong clustering currently on 
> spi/pmci differentiation. That implies a one define applies to multiple 
> board types so naming it, e.g., after a single board type seems not much 
> better than the current approach.

I think it is better to name after one of the board type among all its
supported types. At least it clearly indicates they are related to board
type.

Actually it is normal for many driver modules. A driver was initially
implemented for one board type, and was named by the initial board.
But later you have more board types compatible to the driver, you don't
change the driver name, just use it.

Thanks,
Yilun

> 
> I've even thought myself of removing those defines as they seem one-time 
> use ones after introducing the csr_map. Defining the csr_map using members
> kinda documents what a literal is about if I'd put just a number there.
> The added benefit a few capital letters in a define provides is IMHO very
> questionable.
> 
> Also m10bmc_spi_csr_map name suffers from the same problem, BTW.
> 
> I could, of course now that they're downscoped, drop _SPI_ or _PMCI_ from 
> their names if that's ok for you? ...But that wouldn't address the next 
> version naming problem at all. But I don't anyway know, without a crystal 
> ball, know how to address the future naming needs.
> 
> -- 
>  i.