Hi Arnd, On Sat, Jan 25, 2014 at 2:23 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Friday 24 January 2014, Pratyush Anand wrote: > > On Thu, Jan 23, 2014 at 08:22:54PM +0800, Arnd Bergmann wrote: > > > 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. > > > > > +/* SPEAr1340 Registers */ > > > > +/* Power Management Registers */ [...] > > > 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. > > > 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@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
