> -----Original Message----- > From: Peter Rosin <p...@axentia.se> > Sent: Monday, February 18, 2019 5:39 PM > To: Leo Li <leoyang...@nxp.com>; Pankaj Bansal <pankaj.ban...@nxp.com>; > linux-kernel@vger.kernel.org; Philipp Zabel <p.za...@pengutronix.de> > Subject: Re: [PATCH] drivers: mux: Generic register bitfield-based > multiplexer driver > > On 2019-02-18 22:07, Leo Li wrote: > > From: Peter Rosin <p...@axentia.se> > >> On 2019-02-18 11:20, Pankaj Bansal wrote: > >>> From: Peter Rosin [mailto:p...@axentia.se] > >>>> Anyway, I would prefer if you could extend drivers/mux/mmio.c to > >>>> support both compatibles, and using the compatible to select if > >>>> > >>>> regmap = syscon_node_to_regmap(np->parent); > >>>> > >>>> or > >>>> > >>>> regmap = dev_get_regmap(dev->parent, NULL); > >>>> > >>>> is called to get to the desired regmap. > >>> > >>> This can be done. The name mmio.c however suggests that mux is > >>> controlled by a Memory mapped device. > >>> IMO, if the generic regmap API is to be added to it, the name needs > >>> to changed. Any suggestions ? > >> > >> Just keep the driver name as is, there is no shortage of drivers > >> supporting more than one thing... > > > > You are right that a lot of drivers support multiple functions. But > > the problem here is that the current name mmio is only a subset of > > what the updated driver will handle, which can create confusion. > > I refuse the duplication. This new driver is doing the exact same thing (-ish) > as the old one. Having the same code in two places is just a recipe for future > divergence when everyone have forgotten that there are two nearly > identical drivers that both need patching. Stating this in a comment > somewhere in the drivers will not help all that much in my experience. The > comment will be missing from the context in some seemingly trivial patch, > and there you go. There *will* be weed down the line, if duplication is > allowed.
I agree that we should avoid the duplication. > > I can agree that mux-regmap.c and CONFIG_MUX_REGMAP would have > been better names, but mux-mmio is already there. So it is what it is. But if > you can convince me that changing the name will not cause any trouble > anywhere for any existing mux-mmio users, I suppose we can do a rename. > But I bet there will be some nasty corner cases and odd use cases, so you will > have to present strong arguments. I don't think that it is hard to maintain the backward compatibility with the rename. The updated driver can keep handling the "mmio-mux" device tree compatible string. And we can also have MUX_MMIO selects the new MUX_REGMAP if we want to keep the compatibility with old kernel config file. > > Just update the Kconfig to document the dual nature and remove the > MFD_SYSCON dependency. I suppose you also need to handle the possibly > missing syscon in the .c file, details, details. But something like this > perhaps: > > config MUX_MMIO > tristate "MMIO/Regmap register bitfield-controlled Multiplexer" > depends on OF || COMPILE_TEST > help > MMIO/Regmap register bitfield-controlled Multiplexer controller. > > The driver builds multiplexer controllers for bitfields in either > a syscon register or a driver regmap register. For N bit wide > bitfields, there will be 2^N possible multiplexer states. > > To compile the driver as a module, choose M here: the module will > be called mux-mmio. > > Cheers, > Peter