Re: [PATCH] i2c: devtree-aware iic support for PPC4xx

["Grant Likely" <grant.likely@secretlab.ca>]

On 9/16/07, Stefan Roese <sr@denx.de> wrote:
> On Sunday 16 September 2007, Eugene Surovegin wrote:
> > Hmm, I just noticed that you basically added a copy of existing
> > driver with small changes to support OF while keeping OCP one.
> >
> > Why not just add OF support to the existing code (under some ifdef),
> > and then remove OCP support as soon as ppc -> powerpc transition is
> > finished? Why have two almost identical code in the tree?
>
> My understanding was, that adding many #ifdef's into the code was not the
> preferred way. I could of course change this patch to not add an additional
> driver but extend the existing driver with a bunch of #ifdef's to support
> both versions.
>
> This approach of multiple drivers seems to be common in the kernel right now:
>
> drivers/mtd/maps/physmap.c
> drivers/mtd/maps/physmap_of.c
>
> or
>
> drivers/usb/host/ohci-ppc-soc.c
> drivers/usb/host/ohci-ppc-of.c
>
> Any other opinions on this? How should this be handled to get accepted
> upstream? Two different drivers with removing the "old" one later when
> arch/ppc is gone, or one driver which supports both versions and removing the
> ocp support in this driver later?

FWIW, I'm not a big fan of duplicating drivers, and I don't think it
should be necessary.  If a large number of #ifdef block are needed
then it may be that it would be better to refactor parts of the driver
to localize the conditionally compiled code.

Also, neither of these examples are particularly strong support.  In
both cases (physmap and ohci-ppc) the mentioned C files are bus
mappings only, not the actual driver.  In this scenario it does make
sense to use separate files, but it requires the dependent driver to
be abstracted enough to support multiple bindings.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195