On Mon, Nov 19, 2018 at 07:44:14PM +0100, Andreas Kemnade wrote: > On Mon, 19 Nov 2018 09:22:59 +0100 > "H. Nikolaus Schaller" <hns@xxxxxxxxxxxxx> wrote: > > > Am 18.11.2018 um 22:57 schrieb Andreas Kemnade <andreas@xxxxxxxxxxxx>: > > > > > > Here is another chapter of the story to get gta04 gnss power > > > management into the mainline kernel. > > > There is a w2sg0004 without wakeup line in there, so power state > > > can only be determined indirectly by looking at the serial data lines. > > > Then there as also an lna which needs to be powered for real gps > > > reception. That part needs probably more discussion, since it might > > > be an idea to have it more generalized since it has nothing todo > > > with the chip itself. > > > > On the other hand if we follow the "SoC is the spider in the net" > > way of looking at DTS hierarchy, we have the uart as a child of the > > SoC and the gnss receiver as a serdev child of the UART. The LNA > > is even one step more distantly connected to the gnss. So it makes > > sense to me to have it as a property/reference of the gnss chip's > > DTS record which is a sibling of the compatible records. So the only > > place where it can be reasonably processed is the driver. > > > Or the lna is a child of the gnss receiver. The whole thing > should probably not be overdesigned, but it does not make sense that > every gnss chip driver has some lna logic. Did you mean "does make sense" here? > Maybe the regulator should just be stored in the struct > gnss_device and then drivers/gnss/core.c takes care. Maybe eventually, but keeping it per driver is fine for now. As you say above, this really isn't part of the chip itself, and therefore should probably be a generic gnss property as it could be required for any receiver (in principle). But we still need driver support for coordinating it with the rest of the receiver power management, so adding it to drivers as need arises seems reasonable. Thanks, Johan