[PATCH V2 4/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to system cfg driver

Hi Arnd,


On Sat, Jan 25, 2014 at 2:23 AM, Arnd Bergmann [off-list ref] wrote:

On Friday 24 January 2014, Pratyush Anand wrote:

quoted

On Thu, Jan 23, 2014 at 08:22:54PM +0800, Arnd Bergmann wrote:

quoted

On Thursday 23 January 2014, Mohit Kumar wrote:

I assume you'd want a phandle pointing to the syscon device in here
as well?

Since there is only one syscon device in the whole DT, so do I really
need to add phandle? Currently I am using
syscon_regmap_lookup_by_compatible to find  syscon device.

I'd much rather use syscon_regmap_lookup_by_phandle than
syscon_regmap_lookup_by_compatible, all the time, since this makes
the relationship between the devices explicit.

The phandle method also allows you to pass regmap indexes in the
same property, which can be handy if two variants of the chip have
the same registers at a different offset.

quoted

quoted

quoted

+/* SPEAr1340 Registers */
+/* Power Management Registers */

[...]

quoted

quoted

Looking at the actual code now, this very much looks like it ought to
be a "phy" driver and get put in drivers/phy/.

Actually these registers are part of common system configurations
register space (called as misc space) for SPEAr SOC. So we opted for
syscon framework.

The use of syscon for this is good, I have no objection to that, and
was suggesting that you create a logical "phy" device that uses the
misc syscon device as a backend.

quoted

PHY registers space starts from 0xEB800000, which can be
programmed for various phy specific functions like power management,
tx/rx settings, comparator settings etc. In most of the cases phy
works with default settings, however there are few exceptions for
which we will be adding a phy driver for further improvement of SPEAr
drivers.

I see. So while the code you have here could be expressed as a phy driver
by itself, there is another part of the SoC that controls the actual
phy. How about if you add the phy device node to DT, and write a driver
that doesn't actually program the phy registers for now, but does contain
the code that you have posted here. That would give you flexibility for
future extensions and at the same time let you remove all SPEAr specific
code from the actual AHCI driver by using the generic ahci-platform
driver.

OK..
--  will move all these code to a phy driver.
-- so, no need of a new cfg node as of now.
-- will pass syscon phandle to phy driver.
-- currently will keep ahci platform plugin (as it is here) in phy driver.
will remove when generic ahci driver is in place.

Regards
Pratyush

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel