Re: [RFC 1/4] ARM: tegra: Move SoC drivers to drivers/soc/tegra

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 16, 2014 at 12:31:00PM -0700, Olof Johansson wrote:
> [ Gee, I had completely missed this thread because nobody bothered to
> cc me. Seems to be standard procedure on 64-bit these days. :( ]
> 
> 
> On Mon, Jun 30, 2014 at 12:20 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > On Saturday 28 June 2014 22:40:26 Thierry Reding wrote:
> >> > > pmc.c implements various routines to access the power management
> >> > > controller, some of which is needed by suspend/resume, some of it is
> >> > > needed by SMP. powergate.c implements a subset of the PMC that needs to
> >> > > be exported to drivers to enable power partitions on the SoC. I'm not
> >> > > aware of subsystems that deal with this kind of driver.
> >> > >
> >> > Please see above.
> >>
> >> Like I said, I don't see what business suspend/resume related code has
> >> in drivers/power. What we're talking about here really is functionality
> >> very specific to Tegra. Also some of that code needs to be run at very
> >> early points in the boot process, so we can't reasonably turn it into a
> >> proper driver anyway.
> >
> > I believe the powergate.c stuff can be changed into pm_domain code, but
> > we don't have a good subsystem with generic DT bindings yet, so that
> > may need some more groundwork first. drivers/power or a subdirectory
> > of that may end up being the right place though.
> >
> >> Also, the real win we get from moving code out into drivers/ is if we
> >> can provide a common framework for them. I don't see how we can possibly
> >> do that for this code since there simply isn't enough commonality
> >> between SoCs. At the same time we now have a situation where both 32-bit
> >> and 64-bit variants of some SoCs share some of the same hardware at the
> >> very low level and since we don't have mach-* directories for 64-bit ARM
> >> and shouldn't be duplicating code either, we need to find a new home for
> >> this type of code. drivers/soc seemed to fit perfectly well.
> >
> > For the low-level stuff yes, but I agree with Santosh there needs to be
> > some more work trying to split out individual high-level drivers.
> >
> > There are two common patterns for the interface between the low-level
> > register access and the more high-level stuff:  you can either use
> > a "syscon" driver that just exports a struct regmap and that gets referenced
> > from other drivers using a phandle in their device nodes, or you have
> > a driver in drivers/soc that exports a somewhat higher-level interface
> > and comes with its own header file that gets included by other drivers.
> > At the moment, the syscon/regmap variant can only be used once device
> > drivers are loaded, but there is some broad agreement that it should
> > be changed to allow calling syscon_regmap_lookup_by_phandle() at
> > early boot using just DT accessors.
> 
> I'd strongly prefer to NOT tie this into DT and keep it as in-kernel
> implementation until we know more what a common subsystem might look
> like, if any.
> 
> If we keep it only in the kernel, then we're free to change it as
> needed. If we tie it into DT/syscon, then we get stick with
> stable-only ABIs from day one. That doesn't seem like a good idea.

There are two sides to keeping to an in-kernel API too: it may seem like
we're free to change this at will and aren't subject to a stable ABI.
However that's not entirely accurate.

Assume we don't put any information about a feature into DT but use an
in-kernel API only. Once we've come up with a good way to represent that
in DT and have a generic framework that we can use, we'll most likely
still need to keep around the old API in the kernel to make sure the
kernel still works with a DT that doesn't have nodes and/or properties
for the new binding.

So the question arises: at that point is it really worth using a DT
binding? Since we already have an API that works, why bother coming up
with a new one? Especially if we can't get rid of the one we already
have anyway?

Thierry

Attachment: pgpllhK1ZQ6_o.pgp
Description: PGP signature


[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux