On Thu, Oct 31, 2019 at 3:00 AM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote: > > > > On 30/10/2019 20.49, Rob Herring wrote: > > On Wed, Oct 30, 2019 at 9:30 AM Peter Ujfalusi <peter.ujfalusi@xxxxxx> wrote: > >> > >> > >> > >> On 30/10/2019 16.17, Mark Brown wrote: > >>> On Wed, Oct 30, 2019 at 03:32:09PM +0200, Peter Ujfalusi wrote: > >>>> On 30/10/2019 15.12, Rob Herring wrote: > >>> > >>>>> Why can't we just add a shared flag like we have for interrupts? > >>>>> Effectively, we have that for resets too, it's just hardcoded in the > >>>>> the drivers. > >>> > >>>> This would be kind of the same thing what the > >>>> GPIOD_FLAGS_BIT_NONEXCLUSIVE does, which was a quick workaround for > >>>> fixed-regulators afaik. > >>> > >>> The theory with that was that any usage of this would need the > >>> higher level code using the GPIO to cooperate so they didn't step > >>> on each other's toes so the GPIO code should just punt to it. > >> > >> But from the client driver point of view a GPIO is still GPIO and if the > >> components are unrelated then it is hard to patch things together from > >> the top. > > > > You can't escape a driver being aware. If a driver depends on that > > GPIO to actually be set to states the driver says, then it can't be > > guaranteed to work. For example, maybe the driver assumes the device > > is in reset state after toggling reset and doesn't work if not in > > reset state. The driver has to be aware no matter what you do in DT. > > That's true for some device, but it is also true that some can not > tolerate being reset without them knowing it. You mean a reset when the driver is not loaded would not work? How could that ever work? I don't think you can have any reset control in the drivers in that case. > If all users of the shared GPIO have full control over it then they can > just toggle it whatever way they want. How would a regulator, codec, > amplifier would negotiate on what to do with the shared GPIO? > > Another not uncommon setup is when the two components needs different level: > C1: ENABLE is high active > C2: RESET is high active > > To enable C1, the GPIO should be high. To enable C2 the GPIO must be low. > In the board one of the branch of the shared GPIO needs (and have) a > logic inverter. > > If they both control the same GPIO then they must have requested it with > different GPIO_ACTIVE_ since the drivers are written according to chip > spec, so C1 sets the GPIO to 1, C2 sets it to 0, the inversion for one > of them must happen in gpio core, right? No, drivers are written to set the state to active/inactive. The DT GPIO_ACTIVE_ flags can depend on an inverter being present (BTW, there was a recent attempt to do an inverter binding). > It should be possible to add pass-through mode for gpio-shared so that > all requests would propagate to the root GPIO if that's what needed for > some setups. > > That way the gpio-shared would nicely handle the GPIO inversions, would > be able to handle cases to avoid unwanted reset/enable of components or > allow components to be ninja-reset. What does ninja-reset mean? > I think it would be possible to add gpiod_is_shared(struct gpio_desc > *desc) so users can check if the GPIO is shared - it would only return > true if the gpio-shared is not in pass-through mode so they can know > that the state they see on their gpio desc is not necessary matching > with reality. > Probably another gpiod_shared_get_root_value() to fetch the root's state? > > I intentionally not returning that in the driver as clients might skip a > gpio_set_value() seeing that the GPIO line is already in a state they > would want it, but that would not register their needs for the level. > > - Péter > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
