Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Fri, Sep 27, 2013 at 06:35:31AM -0700, Mike Dunn wrote: > On 09/26/2013 05:50 AM, Thierry Reding wrote: > > On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: > >> On 26/09/13 15:02, Thierry Reding wrote: > >>> On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: > On 11/09/13 14:40, Mike Dunn wrote: > > On 09/10/2013 10:21 AM, Thierry Reding wrote: > > >> Do you have a real setup that actually needs multiple GPIOs? Usually > >> such a setup requires some kind of timing or other additional > >> constraint > >> which can't be represented by this simple binding. > >> > >> Looking at the Palm Treo code it seems like the reason why multiple > >> GPIOs are needed is because one is to enable the backlight, while the > >> other is in fact used to enable the LCD panel. > > > > > > There are actually four GPIOs involved! (There is an embarrasingly > > horrible > > hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is > > almost > > certainly simply backlight power. The other three are probably LCD > > related. > > When you say "power", do you mean the gpio enables a regulator to feed > power to the backlight? If so, wouldn't that be a regulator, not gpio, > from the bl driver's point of view? > > Generally speaking, this same problem appears in many places, but for > some reason especially in display. I'm a bit hesitant in adding "free > form" gpio/regulator support for drivers, as, as Thierry pointed out, > there are often timing requirements, or sometimes the gpios are > inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll > assert the gpio for a short moment only. > >>> > >>> I sent out another series a few days ago that somewhat obsoletes this > >>> patch. What it does is basically add a single enable GPIO that can be > >>> used to turn the backlight on and off. In a separate patch, support is > >>> added for a power regulator. The combination of both should be able to > >>> cover the majority of use-cases. > >> > >> But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and > >> one regulator? > > > > Well, at least for the backlight it only seemed to involve a single > > GPIO. The other three were probably related to LCD and therefore not > > really suitable for a backlight driver. Traditionally it has been that > > the backlight driver handled these things as well (via the callbacks > > installed by board setup code). While really they should be handled by a > > separate driver (for the LCD). > > > Yes, this is currently my best guess. This is reverse-engineered and > unfortunately I'm not yet able to accurately describe my particular use-case. > Probably as wacky as anything you can imagine, Thierry :) > > The gpio and regulator patches will probably suffice. Thierry, can you please > point me to those patches? I don't see them in your gitorious tree. If they > were posted to linux-pwm, I missed them; sorry. I've stumbled across this email and it's not marked as answered, so here goes: these patches will be part of my pull request for 3.13. They should now be in my tree, although that's now moved to kernel.org. You can find it here: https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git Thierry pgpOyXmwBGnN1.pgp Description: PGP signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Fri, Sep 27, 2013 at 06:35:31AM -0700, Mike Dunn wrote: On 09/26/2013 05:50 AM, Thierry Reding wrote: On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: On 26/09/13 15:02, Thierry Reding wrote: On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and one regulator? Well, at least for the backlight it only seemed to involve a single GPIO. The other three were probably related to LCD and therefore not really suitable for a backlight driver. Traditionally it has been that the backlight driver handled these things as well (via the callbacks installed by board setup code). While really they should be handled by a separate driver (for the LCD). Yes, this is currently my best guess. This is reverse-engineered and unfortunately I'm not yet able to accurately describe my particular use-case. Probably as wacky as anything you can imagine, Thierry :) The gpio and regulator patches will probably suffice. Thierry, can you please point me to those patches? I don't see them in your gitorious tree. If they were posted to linux-pwm, I missed them; sorry. I've stumbled across this email and it's not marked as answered, so here goes: these patches will be part of my pull request for 3.13. They should now be in my tree, although that's now moved to kernel.org. You can find it here: https://git.kernel.org/cgit/linux/kernel/git/thierry.reding/linux-pwm.git Thierry pgpOyXmwBGnN1.pgp Description: PGP signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 09/26/2013 05:50 AM, Thierry Reding wrote: > On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: >> On 26/09/13 15:02, Thierry Reding wrote: >>> On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: > On 09/10/2013 10:21 AM, Thierry Reding wrote: >> Do you have a real setup that actually needs multiple GPIOs? Usually >> such a setup requires some kind of timing or other additional constraint >> which can't be represented by this simple binding. >> >> Looking at the Palm Treo code it seems like the reason why multiple >> GPIOs are needed is because one is to enable the backlight, while the >> other is in fact used to enable the LCD panel. > > > There are actually four GPIOs involved! (There is an embarrasingly > horrible > hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is > almost > certainly simply backlight power. The other three are probably LCD > related. When you say "power", do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding "free form" gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. >>> >>> I sent out another series a few days ago that somewhat obsoletes this >>> patch. What it does is basically add a single enable GPIO that can be >>> used to turn the backlight on and off. In a separate patch, support is >>> added for a power regulator. The combination of both should be able to >>> cover the majority of use-cases. >> >> But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and >> one regulator? > > Well, at least for the backlight it only seemed to involve a single > GPIO. The other three were probably related to LCD and therefore not > really suitable for a backlight driver. Traditionally it has been that > the backlight driver handled these things as well (via the callbacks > installed by board setup code). While really they should be handled by a > separate driver (for the LCD). Yes, this is currently my best guess. This is reverse-engineered and unfortunately I'm not yet able to accurately describe my particular use-case. Probably as wacky as anything you can imagine, Thierry :) The gpio and regulator patches will probably suffice. Thierry, can you please point me to those patches? I don't see them in your gitorious tree. If they were posted to linux-pwm, I missed them; sorry. Thanks, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 26/09/13 15:50, Thierry Reding wrote: >> I thought the NAK was for the DT parts, not for the sequences as such. I >> don't remember anyone shooting down the idea of defining power sequences >> inside a driver. > > Yes, but the DT parts were the primary reason why they were written in > the first place. Without DT we can just use the existing hooks to do the > sequencing. There is not much to be gained from power sequences. Board hooks? Those cannot be used with DT boot. But yes, the power sequences without DT (or platform data) parts cannot handle with board-specific-things. I still think it'd be a very nice thing to have inside the drivers. A single driver could more easily handle bunch of somewhat similar devices, by specifying power sequences in a table, one sequence for each device. > There is unfortunately always the next crazy setup that one can think > of. I personally prefer to support what we have (or at least the > majority of that) with something generic and tackle the more exotic > setups later on (or when they appear, as the case may be). > > As things stand right now, there's no way to get the simplest panel > setup to work properly if you use DT. By adding both an enable GPIO and > a power supply regulator we can at least cover the sane use-cases with > some sane and pretty simple code. Sure, no disagreement there. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 26/09/13 15:50, Thierry Reding wrote: I thought the NAK was for the DT parts, not for the sequences as such. I don't remember anyone shooting down the idea of defining power sequences inside a driver. Yes, but the DT parts were the primary reason why they were written in the first place. Without DT we can just use the existing hooks to do the sequencing. There is not much to be gained from power sequences. Board hooks? Those cannot be used with DT boot. But yes, the power sequences without DT (or platform data) parts cannot handle with board-specific-things. I still think it'd be a very nice thing to have inside the drivers. A single driver could more easily handle bunch of somewhat similar devices, by specifying power sequences in a table, one sequence for each device. There is unfortunately always the next crazy setup that one can think of. I personally prefer to support what we have (or at least the majority of that) with something generic and tackle the more exotic setups later on (or when they appear, as the case may be). As things stand right now, there's no way to get the simplest panel setup to work properly if you use DT. By adding both an enable GPIO and a power supply regulator we can at least cover the sane use-cases with some sane and pretty simple code. Sure, no disagreement there. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 09/26/2013 05:50 AM, Thierry Reding wrote: On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: On 26/09/13 15:02, Thierry Reding wrote: On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and one regulator? Well, at least for the backlight it only seemed to involve a single GPIO. The other three were probably related to LCD and therefore not really suitable for a backlight driver. Traditionally it has been that the backlight driver handled these things as well (via the callbacks installed by board setup code). While really they should be handled by a separate driver (for the LCD). Yes, this is currently my best guess. This is reverse-engineered and unfortunately I'm not yet able to accurately describe my particular use-case. Probably as wacky as anything you can imagine, Thierry :) The gpio and regulator patches will probably suffice. Thierry, can you please point me to those patches? I don't see them in your gitorious tree. If they were posted to linux-pwm, I missed them; sorry. Thanks, Mike -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: > On 26/09/13 15:02, Thierry Reding wrote: > > On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: > >> On 11/09/13 14:40, Mike Dunn wrote: > >>> On 09/10/2013 10:21 AM, Thierry Reding wrote: > >> > Do you have a real setup that actually needs multiple GPIOs? Usually > such a setup requires some kind of timing or other additional constraint > which can't be represented by this simple binding. > > Looking at the Palm Treo code it seems like the reason why multiple > GPIOs are needed is because one is to enable the backlight, while the > other is in fact used to enable the LCD panel. > >>> > >>> > >>> There are actually four GPIOs involved! (There is an embarrasingly > >>> horrible > >>> hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is > >>> almost > >>> certainly simply backlight power. The other three are probably LCD > >>> related. > >> > >> When you say "power", do you mean the gpio enables a regulator to feed > >> power to the backlight? If so, wouldn't that be a regulator, not gpio, > >> from the bl driver's point of view? > >> > >> Generally speaking, this same problem appears in many places, but for > >> some reason especially in display. I'm a bit hesitant in adding "free > >> form" gpio/regulator support for drivers, as, as Thierry pointed out, > >> there are often timing requirements, or sometimes the gpios are > >> inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll > >> assert the gpio for a short moment only. > > > > I sent out another series a few days ago that somewhat obsoletes this > > patch. What it does is basically add a single enable GPIO that can be > > used to turn the backlight on and off. In a separate patch, support is > > added for a power regulator. The combination of both should be able to > > cover the majority of use-cases. > > But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and > one regulator? Well, at least for the backlight it only seemed to involve a single GPIO. The other three were probably related to LCD and therefore not really suitable for a backlight driver. Traditionally it has been that the backlight driver handled these things as well (via the callbacks installed by board setup code). While really they should be handled by a separate driver (for the LCD). > > That series doesn't handle any timing requirements, but again, for the > > majority of the setups supported by a power supply and enable GPIO the > > timing doesn't matter. > > > >> I haven't seen new versions for the power sequences framework (without > >> DT support), I think it could help us here a bit by simplifying how we > >> present the sequences inside the driver. But we still need multiple > >> drivers or the same driver supporting multiple devices. > > > > I'm not sure if power sequences will be very helpful here. There was an > > attempt to get those merged, but the patches were NAKed in the end. I'm > > not aware of any work currently being done on them. > > I thought the NAK was for the DT parts, not for the sequences as such. I > don't remember anyone shooting down the idea of defining power sequences > inside a driver. Yes, but the DT parts were the primary reason why they were written in the first place. Without DT we can just use the existing hooks to do the sequencing. There is not much to be gained from power sequences. > >> And I also think that the model where we have just one driver for, say, > >> backlight may not be enough. There may be multiple hardware components, > >> that used together will generate the backlight. And each component > >> requires specific management. > > > > I'm not sure what other components you are talking about here. Can you > > elaborate? > > I don't have any specific case in mind, and maybe these are quite rare. > But there could be some kind of mix of muxes, regulators, gpios and > whatnot that need to be managed in certain way to make backlight (or > display) work. > > I'm making this up, so I'm not sure if this is sensible, but consider a > board where there's a mux to select where a backlight gets its PWM > input, and the mux is controlled via i2c. And maybe insert some kind of > level shifter in between, enabled with a GPIO. And some third component, > as this hypothetical board is a development board, and hardware people > seem to love to make bizarre designs, that work in theory but the SW is > almost impossible to design... > > So to enable the backlight, we might need to do multiple different > things, depending on which components this particular board has. > Especially for a mux controlled via i2c it would make sense to have a > separate driver. Having just a single backlight driver might not be enough. > > Sometimes, or hopefully often, that kind of complexity can be hidden > behind common frameworks. For example, enabling a gpio could result in > the gpio
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 26/09/13 15:02, Thierry Reding wrote: > On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: >> On 11/09/13 14:40, Mike Dunn wrote: >>> On 09/10/2013 10:21 AM, Thierry Reding wrote: >> Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. >>> >>> >>> There are actually four GPIOs involved! (There is an embarrasingly horrible >>> hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is >>> almost >>> certainly simply backlight power. The other three are probably LCD related. >> >> When you say "power", do you mean the gpio enables a regulator to feed >> power to the backlight? If so, wouldn't that be a regulator, not gpio, >> from the bl driver's point of view? >> >> Generally speaking, this same problem appears in many places, but for >> some reason especially in display. I'm a bit hesitant in adding "free >> form" gpio/regulator support for drivers, as, as Thierry pointed out, >> there are often timing requirements, or sometimes the gpios are >> inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll >> assert the gpio for a short moment only. > > I sent out another series a few days ago that somewhat obsoletes this > patch. What it does is basically add a single enable GPIO that can be > used to turn the backlight on and off. In a separate patch, support is > added for a power regulator. The combination of both should be able to > cover the majority of use-cases. But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and one regulator? > That series doesn't handle any timing requirements, but again, for the > majority of the setups supported by a power supply and enable GPIO the > timing doesn't matter. > >> I haven't seen new versions for the power sequences framework (without >> DT support), I think it could help us here a bit by simplifying how we >> present the sequences inside the driver. But we still need multiple >> drivers or the same driver supporting multiple devices. > > I'm not sure if power sequences will be very helpful here. There was an > attempt to get those merged, but the patches were NAKed in the end. I'm > not aware of any work currently being done on them. I thought the NAK was for the DT parts, not for the sequences as such. I don't remember anyone shooting down the idea of defining power sequences inside a driver. > But as I said above, the combination of an enable GPIO and power supply > should be enough to get a lot of use-cases supported. Yep. >> And I also think that the model where we have just one driver for, say, >> backlight may not be enough. There may be multiple hardware components, >> that used together will generate the backlight. And each component >> requires specific management. > > I'm not sure what other components you are talking about here. Can you > elaborate? I don't have any specific case in mind, and maybe these are quite rare. But there could be some kind of mix of muxes, regulators, gpios and whatnot that need to be managed in certain way to make backlight (or display) work. I'm making this up, so I'm not sure if this is sensible, but consider a board where there's a mux to select where a backlight gets its PWM input, and the mux is controlled via i2c. And maybe insert some kind of level shifter in between, enabled with a GPIO. And some third component, as this hypothetical board is a development board, and hardware people seem to love to make bizarre designs, that work in theory but the SW is almost impossible to design... So to enable the backlight, we might need to do multiple different things, depending on which components this particular board has. Especially for a mux controlled via i2c it would make sense to have a separate driver. Having just a single backlight driver might not be enough. Sometimes, or hopefully often, that kind of complexity can be hidden behind common frameworks. For example, enabling a gpio could result in the gpio driver enabling a regulator, sending i2c messages, or whatever. But I don't think that's possible in all cases. Anyway, this was really more of "thinking out loud" than any suggestion. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: > On 11/09/13 14:40, Mike Dunn wrote: > > On 09/10/2013 10:21 AM, Thierry Reding wrote: > > >> Do you have a real setup that actually needs multiple GPIOs? Usually > >> such a setup requires some kind of timing or other additional constraint > >> which can't be represented by this simple binding. > >> > >> Looking at the Palm Treo code it seems like the reason why multiple > >> GPIOs are needed is because one is to enable the backlight, while the > >> other is in fact used to enable the LCD panel. > > > > > > There are actually four GPIOs involved! (There is an embarrasingly horrible > > hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is > > almost > > certainly simply backlight power. The other three are probably LCD related. > > When you say "power", do you mean the gpio enables a regulator to feed > power to the backlight? If so, wouldn't that be a regulator, not gpio, > from the bl driver's point of view? > > Generally speaking, this same problem appears in many places, but for > some reason especially in display. I'm a bit hesitant in adding "free > form" gpio/regulator support for drivers, as, as Thierry pointed out, > there are often timing requirements, or sometimes the gpios are > inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll > assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. That series doesn't handle any timing requirements, but again, for the majority of the setups supported by a power supply and enable GPIO the timing doesn't matter. > I haven't seen new versions for the power sequences framework (without > DT support), I think it could help us here a bit by simplifying how we > present the sequences inside the driver. But we still need multiple > drivers or the same driver supporting multiple devices. I'm not sure if power sequences will be very helpful here. There was an attempt to get those merged, but the patches were NAKed in the end. I'm not aware of any work currently being done on them. But as I said above, the combination of an enable GPIO and power supply should be enough to get a lot of use-cases supported. > And I also think that the model where we have just one driver for, say, > backlight may not be enough. There may be multiple hardware components, > that used together will generate the backlight. And each component > requires specific management. I'm not sure what other components you are talking about here. Can you elaborate? Thierry pgpNXGSyXPUDT.pgp Description: PGP signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 11/09/13 14:40, Mike Dunn wrote: > On 09/10/2013 10:21 AM, Thierry Reding wrote: >> Do you have a real setup that actually needs multiple GPIOs? Usually >> such a setup requires some kind of timing or other additional constraint >> which can't be represented by this simple binding. >> >> Looking at the Palm Treo code it seems like the reason why multiple >> GPIOs are needed is because one is to enable the backlight, while the >> other is in fact used to enable the LCD panel. > > > There are actually four GPIOs involved! (There is an embarrasingly horrible > hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost > certainly simply backlight power. The other three are probably LCD related. When you say "power", do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding "free form" gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I haven't seen new versions for the power sequences framework (without DT support), I think it could help us here a bit by simplifying how we present the sequences inside the driver. But we still need multiple drivers or the same driver supporting multiple devices. And I also think that the model where we have just one driver for, say, backlight may not be enough. There may be multiple hardware components, that used together will generate the backlight. And each component requires specific management. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I haven't seen new versions for the power sequences framework (without DT support), I think it could help us here a bit by simplifying how we present the sequences inside the driver. But we still need multiple drivers or the same driver supporting multiple devices. And I also think that the model where we have just one driver for, say, backlight may not be enough. There may be multiple hardware components, that used together will generate the backlight. And each component requires specific management. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. That series doesn't handle any timing requirements, but again, for the majority of the setups supported by a power supply and enable GPIO the timing doesn't matter. I haven't seen new versions for the power sequences framework (without DT support), I think it could help us here a bit by simplifying how we present the sequences inside the driver. But we still need multiple drivers or the same driver supporting multiple devices. I'm not sure if power sequences will be very helpful here. There was an attempt to get those merged, but the patches were NAKed in the end. I'm not aware of any work currently being done on them. But as I said above, the combination of an enable GPIO and power supply should be enough to get a lot of use-cases supported. And I also think that the model where we have just one driver for, say, backlight may not be enough. There may be multiple hardware components, that used together will generate the backlight. And each component requires specific management. I'm not sure what other components you are talking about here. Can you elaborate? Thierry pgpNXGSyXPUDT.pgp Description: PGP signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 26/09/13 15:02, Thierry Reding wrote: On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and one regulator? That series doesn't handle any timing requirements, but again, for the majority of the setups supported by a power supply and enable GPIO the timing doesn't matter. I haven't seen new versions for the power sequences framework (without DT support), I think it could help us here a bit by simplifying how we present the sequences inside the driver. But we still need multiple drivers or the same driver supporting multiple devices. I'm not sure if power sequences will be very helpful here. There was an attempt to get those merged, but the patches were NAKed in the end. I'm not aware of any work currently being done on them. I thought the NAK was for the DT parts, not for the sequences as such. I don't remember anyone shooting down the idea of defining power sequences inside a driver. But as I said above, the combination of an enable GPIO and power supply should be enough to get a lot of use-cases supported. Yep. And I also think that the model where we have just one driver for, say, backlight may not be enough. There may be multiple hardware components, that used together will generate the backlight. And each component requires specific management. I'm not sure what other components you are talking about here. Can you elaborate? I don't have any specific case in mind, and maybe these are quite rare. But there could be some kind of mix of muxes, regulators, gpios and whatnot that need to be managed in certain way to make backlight (or display) work. I'm making this up, so I'm not sure if this is sensible, but consider a board where there's a mux to select where a backlight gets its PWM input, and the mux is controlled via i2c. And maybe insert some kind of level shifter in between, enabled with a GPIO. And some third component, as this hypothetical board is a development board, and hardware people seem to love to make bizarre designs, that work in theory but the SW is almost impossible to design... So to enable the backlight, we might need to do multiple different things, depending on which components this particular board has. Especially for a mux controlled via i2c it would make sense to have a separate driver. Having just a single backlight driver might not be enough. Sometimes, or hopefully often, that kind of complexity can be hidden behind common frameworks. For example, enabling a gpio could result in the gpio driver enabling a regulator, sending i2c messages, or whatever. But I don't think that's possible in all cases. Anyway, this was really more of thinking out loud than any suggestion. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Thu, Sep 26, 2013 at 03:26:13PM +0300, Tomi Valkeinen wrote: On 26/09/13 15:02, Thierry Reding wrote: On Thu, Sep 26, 2013 at 01:13:18PM +0300, Tomi Valkeinen wrote: On 11/09/13 14:40, Mike Dunn wrote: On 09/10/2013 10:21 AM, Thierry Reding wrote: Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. When you say power, do you mean the gpio enables a regulator to feed power to the backlight? If so, wouldn't that be a regulator, not gpio, from the bl driver's point of view? Generally speaking, this same problem appears in many places, but for some reason especially in display. I'm a bit hesitant in adding free form gpio/regulator support for drivers, as, as Thierry pointed out, there are often timing requirements, or sometimes the gpios are inverted, or sometimes the gpio is, in fact, a reset gpio, where you'll assert the gpio for a short moment only. I sent out another series a few days ago that somewhat obsoletes this patch. What it does is basically add a single enable GPIO that can be used to turn the backlight on and off. In a separate patch, support is added for a power regulator. The combination of both should be able to cover the majority of use-cases. But Mike's case required 4 GPIOs? Or can that be reduced to one gpio and one regulator? Well, at least for the backlight it only seemed to involve a single GPIO. The other three were probably related to LCD and therefore not really suitable for a backlight driver. Traditionally it has been that the backlight driver handled these things as well (via the callbacks installed by board setup code). While really they should be handled by a separate driver (for the LCD). That series doesn't handle any timing requirements, but again, for the majority of the setups supported by a power supply and enable GPIO the timing doesn't matter. I haven't seen new versions for the power sequences framework (without DT support), I think it could help us here a bit by simplifying how we present the sequences inside the driver. But we still need multiple drivers or the same driver supporting multiple devices. I'm not sure if power sequences will be very helpful here. There was an attempt to get those merged, but the patches were NAKed in the end. I'm not aware of any work currently being done on them. I thought the NAK was for the DT parts, not for the sequences as such. I don't remember anyone shooting down the idea of defining power sequences inside a driver. Yes, but the DT parts were the primary reason why they were written in the first place. Without DT we can just use the existing hooks to do the sequencing. There is not much to be gained from power sequences. And I also think that the model where we have just one driver for, say, backlight may not be enough. There may be multiple hardware components, that used together will generate the backlight. And each component requires specific management. I'm not sure what other components you are talking about here. Can you elaborate? I don't have any specific case in mind, and maybe these are quite rare. But there could be some kind of mix of muxes, regulators, gpios and whatnot that need to be managed in certain way to make backlight (or display) work. I'm making this up, so I'm not sure if this is sensible, but consider a board where there's a mux to select where a backlight gets its PWM input, and the mux is controlled via i2c. And maybe insert some kind of level shifter in between, enabled with a GPIO. And some third component, as this hypothetical board is a development board, and hardware people seem to love to make bizarre designs, that work in theory but the SW is almost impossible to design... So to enable the backlight, we might need to do multiple different things, depending on which components this particular board has. Especially for a mux controlled via i2c it would make sense to have a separate driver. Having just a single backlight driver might not be enough. Sometimes, or hopefully often, that kind of complexity can be hidden behind common frameworks. For example, enabling a gpio could result in the gpio driver enabling a regulator, sending i2c messages, or whatever. But I don't think that's possible in all cases. Anyway, this was really more of thinking out loud than any suggestion. There is
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 09/10/2013 10:21 AM, Thierry Reding wrote: > On Tue, Sep 03, 2013 at 12:26:12PM -0700, Mike Dunn wrote: >> This patch adds support for controlling an arbitrary number of gpios to the >> pwm-backlight driver. This was left as a TODO when initial device tree >> support >> was added by Thierry a while back. This functionality replaces the callbacks >> that are passed in the platform data for non-DT cases. Users can avail >> themselves of this feature by adding a 'gpios' property to the 'backlight' >> node. >> When the update_status() callback in backlight_ops runs, the gpios listed in >> the >> property are asserted/deasserted if the specified brightness is >> non-zero/zero. >> >> Tested on a pxa270-based Palm Treo 680. >> >> Signed-off-by: Mike Dunn >> --- >> >> Thanks for looking! >> >> .../bindings/video/backlight/pwm-backlight.txt | 4 + >> drivers/video/backlight/pwm_bl.c | 128 >> ++--- >> 2 files changed, 113 insertions(+), 19 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> index 1e4fc72..4583e68 100644 >> --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt >> @@ -14,6 +14,9 @@ Required properties: >> Optional properties: >>- pwm-names: a list of names for the PWM devices specified in the >> "pwms" property (see PWM binding[0]) >> + - gpios: An arbitrary number of gpios that must be asserted when the >> + backlight is on, and de-asserted when off. They will be >> asserted >> + in the order they appear, and de-asserted in reverse order. > > Do you have a real setup that actually needs multiple GPIOs? Usually > such a setup requires some kind of timing or other additional constraint > which can't be represented by this simple binding. > > Looking at the Palm Treo code it seems like the reason why multiple > GPIOs are needed is because one is to enable the backlight, while the > other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. Timing doesn't seem to be an issue, but the order is important. This is reverse-engineered without any schematics, and honestly, I'm still guessing with regard to how the board is wired. It probably is not appropriate to manage all four gpios in the backlight driver, but at least one needs to be, so I thought I'd go ahead and prepare a patch that adds gpio support. From what you are telling me, I guess my approach was too simplistic. > I have been working on a > patch series to provide simple panel support, specifically to allow the > separation of backlight and panel power sequencing. Would such a method > work for your use-case as well? Sounds like it would, from what I know. > The work that I'm doing is somewhat DRM > centric, and I don't think there's a DRM driver for PXA, but perhaps it > would be a good occasion to look at converting the PXA display drivers > to DRM... =) OK, thanks. I'll look into it. I'm clueless about DRM... All I can say at this point is that I'm not using X windows, and there is not really any hardware acceleration in the pxa's lcd controller. > > That said, I've had a patch similar to yours in a local tree (in fact in > the same branch as the panel work I've been doing) for quite some time, > which allows a single "enable" GPIO to be specified. I haven't gotten > around to sending it out yet, but I'll do that shortly. The patch is a > little more involved because it exposes the GPIO via platform data as > well and therefore has to update a number of board files, too. While > going over the various board files I found only a single board which can > actually make use of the new functionality (which I also converted in > the patch). Any other cases couldn't be implemented by the simple change > so I suspect they can't either using your proposed binding. Really? Are we talking about the callbacks in struct platform_pwm_backlight_data? I thought the majority of them just requested/released a gpio in init()/exit(), and wiggled it in notify(). I must be missing something. > > I've been trying for a while to come up with a way to support more use- > cases, and I keep coming back to the same solution. Since the DT binding > for power sequences was shot down some time ago, we probably have to > represent any kind of sequencing in C code. So instead of trying to fit > everything into a single binding, I think a more maintainable solution > would be to create separate drivers for the more complex use-cases. That > could either be done by using separate compatible values
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On 09/10/2013 10:21 AM, Thierry Reding wrote: On Tue, Sep 03, 2013 at 12:26:12PM -0700, Mike Dunn wrote: This patch adds support for controlling an arbitrary number of gpios to the pwm-backlight driver. This was left as a TODO when initial device tree support was added by Thierry a while back. This functionality replaces the callbacks that are passed in the platform data for non-DT cases. Users can avail themselves of this feature by adding a 'gpios' property to the 'backlight' node. When the update_status() callback in backlight_ops runs, the gpios listed in the property are asserted/deasserted if the specified brightness is non-zero/zero. Tested on a pxa270-based Palm Treo 680. Signed-off-by: Mike Dunn miked...@newsguy.com --- Thanks for looking! .../bindings/video/backlight/pwm-backlight.txt | 4 + drivers/video/backlight/pwm_bl.c | 128 ++--- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt index 1e4fc72..4583e68 100644 --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt @@ -14,6 +14,9 @@ Required properties: Optional properties: - pwm-names: a list of names for the PWM devices specified in the pwms property (see PWM binding[0]) + - gpios: An arbitrary number of gpios that must be asserted when the + backlight is on, and de-asserted when off. They will be asserted + in the order they appear, and de-asserted in reverse order. Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. There are actually four GPIOs involved! (There is an embarrasingly horrible hack in arch/arm/mach-pxa/palmtreo.c that handles the others.) One is almost certainly simply backlight power. The other three are probably LCD related. Timing doesn't seem to be an issue, but the order is important. This is reverse-engineered without any schematics, and honestly, I'm still guessing with regard to how the board is wired. It probably is not appropriate to manage all four gpios in the backlight driver, but at least one needs to be, so I thought I'd go ahead and prepare a patch that adds gpio support. From what you are telling me, I guess my approach was too simplistic. I have been working on a patch series to provide simple panel support, specifically to allow the separation of backlight and panel power sequencing. Would such a method work for your use-case as well? Sounds like it would, from what I know. The work that I'm doing is somewhat DRM centric, and I don't think there's a DRM driver for PXA, but perhaps it would be a good occasion to look at converting the PXA display drivers to DRM... =) OK, thanks. I'll look into it. I'm clueless about DRM... All I can say at this point is that I'm not using X windows, and there is not really any hardware acceleration in the pxa's lcd controller. That said, I've had a patch similar to yours in a local tree (in fact in the same branch as the panel work I've been doing) for quite some time, which allows a single enable GPIO to be specified. I haven't gotten around to sending it out yet, but I'll do that shortly. The patch is a little more involved because it exposes the GPIO via platform data as well and therefore has to update a number of board files, too. While going over the various board files I found only a single board which can actually make use of the new functionality (which I also converted in the patch). Any other cases couldn't be implemented by the simple change so I suspect they can't either using your proposed binding. Really? Are we talking about the callbacks in struct platform_pwm_backlight_data? I thought the majority of them just requested/released a gpio in init()/exit(), and wiggled it in notify(). I must be missing something. I've been trying for a while to come up with a way to support more use- cases, and I keep coming back to the same solution. Since the DT binding for power sequences was shot down some time ago, we probably have to represent any kind of sequencing in C code. So instead of trying to fit everything into a single binding, I think a more maintainable solution would be to create separate drivers for the more complex use-cases. That could either be done by using separate compatible values within the pwm-backlight driver or via completely different drivers. In the latter case we should
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Tue, Sep 03, 2013 at 12:26:12PM -0700, Mike Dunn wrote: > This patch adds support for controlling an arbitrary number of gpios to the > pwm-backlight driver. This was left as a TODO when initial device tree > support > was added by Thierry a while back. This functionality replaces the callbacks > that are passed in the platform data for non-DT cases. Users can avail > themselves of this feature by adding a 'gpios' property to the 'backlight' > node. > When the update_status() callback in backlight_ops runs, the gpios listed in > the > property are asserted/deasserted if the specified brightness is non-zero/zero. > > Tested on a pxa270-based Palm Treo 680. > > Signed-off-by: Mike Dunn > --- > > Thanks for looking! > > .../bindings/video/backlight/pwm-backlight.txt | 4 + > drivers/video/backlight/pwm_bl.c | 128 > ++--- > 2 files changed, 113 insertions(+), 19 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > index 1e4fc72..4583e68 100644 > --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt > @@ -14,6 +14,9 @@ Required properties: > Optional properties: >- pwm-names: a list of names for the PWM devices specified in the > "pwms" property (see PWM binding[0]) > + - gpios: An arbitrary number of gpios that must be asserted when the > + backlight is on, and de-asserted when off. They will be > asserted > + in the order they appear, and de-asserted in reverse order. Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. I have been working on a patch series to provide simple panel support, specifically to allow the separation of backlight and panel power sequencing. Would such a method work for your use-case as well? The work that I'm doing is somewhat DRM centric, and I don't think there's a DRM driver for PXA, but perhaps it would be a good occasion to look at converting the PXA display drivers to DRM... =) That said, I've had a patch similar to yours in a local tree (in fact in the same branch as the panel work I've been doing) for quite some time, which allows a single "enable" GPIO to be specified. I haven't gotten around to sending it out yet, but I'll do that shortly. The patch is a little more involved because it exposes the GPIO via platform data as well and therefore has to update a number of board files, too. While going over the various board files I found only a single board which can actually make use of the new functionality (which I also converted in the patch). Any other cases couldn't be implemented by the simple change so I suspect they can't either using your proposed binding. I've been trying for a while to come up with a way to support more use- cases, and I keep coming back to the same solution. Since the DT binding for power sequences was shot down some time ago, we probably have to represent any kind of sequencing in C code. So instead of trying to fit everything into a single binding, I think a more maintainable solution would be to create separate drivers for the more complex use-cases. That could either be done by using separate compatible values within the pwm-backlight driver or via completely different drivers. In the latter case we should probably think about exporting some of the pwm-backlight functionality so that drivers can easily reuse some of the code. Thierry pgpOsRtuWTl7o.pgp Description: PGP signature
Re: [PATCH] pwm-backlight: add support for device tree gpio control
On Tue, Sep 03, 2013 at 12:26:12PM -0700, Mike Dunn wrote: This patch adds support for controlling an arbitrary number of gpios to the pwm-backlight driver. This was left as a TODO when initial device tree support was added by Thierry a while back. This functionality replaces the callbacks that are passed in the platform data for non-DT cases. Users can avail themselves of this feature by adding a 'gpios' property to the 'backlight' node. When the update_status() callback in backlight_ops runs, the gpios listed in the property are asserted/deasserted if the specified brightness is non-zero/zero. Tested on a pxa270-based Palm Treo 680. Signed-off-by: Mike Dunn miked...@newsguy.com --- Thanks for looking! .../bindings/video/backlight/pwm-backlight.txt | 4 + drivers/video/backlight/pwm_bl.c | 128 ++--- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt index 1e4fc72..4583e68 100644 --- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt +++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt @@ -14,6 +14,9 @@ Required properties: Optional properties: - pwm-names: a list of names for the PWM devices specified in the pwms property (see PWM binding[0]) + - gpios: An arbitrary number of gpios that must be asserted when the + backlight is on, and de-asserted when off. They will be asserted + in the order they appear, and de-asserted in reverse order. Do you have a real setup that actually needs multiple GPIOs? Usually such a setup requires some kind of timing or other additional constraint which can't be represented by this simple binding. Looking at the Palm Treo code it seems like the reason why multiple GPIOs are needed is because one is to enable the backlight, while the other is in fact used to enable the LCD panel. I have been working on a patch series to provide simple panel support, specifically to allow the separation of backlight and panel power sequencing. Would such a method work for your use-case as well? The work that I'm doing is somewhat DRM centric, and I don't think there's a DRM driver for PXA, but perhaps it would be a good occasion to look at converting the PXA display drivers to DRM... =) That said, I've had a patch similar to yours in a local tree (in fact in the same branch as the panel work I've been doing) for quite some time, which allows a single enable GPIO to be specified. I haven't gotten around to sending it out yet, but I'll do that shortly. The patch is a little more involved because it exposes the GPIO via platform data as well and therefore has to update a number of board files, too. While going over the various board files I found only a single board which can actually make use of the new functionality (which I also converted in the patch). Any other cases couldn't be implemented by the simple change so I suspect they can't either using your proposed binding. I've been trying for a while to come up with a way to support more use- cases, and I keep coming back to the same solution. Since the DT binding for power sequences was shot down some time ago, we probably have to represent any kind of sequencing in C code. So instead of trying to fit everything into a single binding, I think a more maintainable solution would be to create separate drivers for the more complex use-cases. That could either be done by using separate compatible values within the pwm-backlight driver or via completely different drivers. In the latter case we should probably think about exporting some of the pwm-backlight functionality so that drivers can easily reuse some of the code. Thierry pgpOsRtuWTl7o.pgp Description: PGP signature
[PATCH] pwm-backlight: add support for device tree gpio control
This patch adds support for controlling an arbitrary number of gpios to the
pwm-backlight driver. This was left as a TODO when initial device tree support
was added by Thierry a while back. This functionality replaces the callbacks
that are passed in the platform data for non-DT cases. Users can avail
themselves of this feature by adding a 'gpios' property to the 'backlight' node.
When the update_status() callback in backlight_ops runs, the gpios listed in the
property are asserted/deasserted if the specified brightness is non-zero/zero.
Tested on a pxa270-based Palm Treo 680.
Signed-off-by: Mike Dunn
---
Thanks for looking!
.../bindings/video/backlight/pwm-backlight.txt | 4 +
drivers/video/backlight/pwm_bl.c | 128 ++---
2 files changed, 113 insertions(+), 19 deletions(-)
diff --git
a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..4583e68 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -14,6 +14,9 @@ Required properties:
Optional properties:
- pwm-names: a list of names for the PWM devices specified in the
"pwms" property (see PWM binding[0])
+ - gpios: An arbitrary number of gpios that must be asserted when the
+ backlight is on, and de-asserted when off. They will be
asserted
+ in the order they appear, and de-asserted in reverse order.
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
@@ -25,4 +28,5 @@ Example:
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
+ gpios = < 77 0>, < 25 1>; /* gpio 25 is active low */
};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1fea627..1e2ab52 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -20,6 +20,12 @@
#include
#include
#include
+#include
+
+struct pwm_bl_gpio {
+ unsigned int gpio;
+ enum of_gpio_flags flags;
+};
struct pwm_bl_data {
struct pwm_device *pwm;
@@ -27,6 +33,8 @@ struct pwm_bl_data {
unsigned intperiod;
unsigned intlth_brightness;
unsigned int*levels;
+ unsigned intnum_gpios;
+ struct pwm_bl_gpio *gpios;
int (*notify)(struct device *,
int brightness);
void(*notify_after)(struct device *,
@@ -94,14 +102,77 @@ static const struct backlight_ops pwm_backlight_ops = {
};
#ifdef CONFIG_OF
+static int pwm_backlight_dt_notify(struct device *dev, int brightness)
+{
+ struct backlight_device *bl = dev_get_drvdata(dev);
+ struct pwm_bl_data *pb = bl_get_data(bl);
+ int i;
+
+ if (brightness) {
+ for (i = 0; i < pb->num_gpios; i++) {
+ if (pb->gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb->gpios[i].gpio, 0);
+ else
+ gpio_set_value(pb->gpios[i].gpio, 1);
+ }
+ return 0;
+ }
+
+ /* de-assert gpios in reverse order, in case this is important */
+ for (i = pb->num_gpios - 1; i >= 0; i--) {
+ if (pb->gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb->gpios[i].gpio, 1);
+ else
+ gpio_set_value(pb->gpios[i].gpio, 0);
+ }
+ return 0;
+}
+
+static void pwm_backlight_dt_exit(struct pwm_bl_data *pb)
+{
+ int i;
+
+ for (i = 0; i < pb->num_gpios; i++)
+ gpio_free(pb->gpios[i].gpio);
+}
+
+static int pwm_backlight_dt_init(struct device *dev, struct pwm_bl_data *pb)
+{
+ int i, j, ret;
+
+ /* request gpios and drive in the inactive state */
+ for (i = 0; i < pb->num_gpios; i++) {
+ char gpio_name[32];
+ unsigned long flags;
+ if (pb->gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ flags = GPIOF_OUT_INIT_LOW;
+ else
+ flags = GPIOF_OUT_INIT_HIGH;
+ snprintf(gpio_name, 32, "%s.%d", dev_name(dev), i);
+ ret = gpio_request_one(pb->gpios[i].gpio, flags, gpio_name);
+ if (ret) {
+ dev_err(dev, "gpio #%d request failed\n", i);
+ goto gpio_err;
+ }
+ }
+ return 0;
+
+ gpio_err:
+ for (j = 0; j < i; j++)
+ gpio_free(pb->gpios[j].gpio);
+ return ret;
+}
+
static int pwm_backlight_parse_dt(struct device *dev,
- struct platform_pwm_backlight_data *data)
+
[PATCH] pwm-backlight: add support for device tree gpio control
This patch adds support for controlling an arbitrary number of gpios to the
pwm-backlight driver. This was left as a TODO when initial device tree support
was added by Thierry a while back. This functionality replaces the callbacks
that are passed in the platform data for non-DT cases. Users can avail
themselves of this feature by adding a 'gpios' property to the 'backlight' node.
When the update_status() callback in backlight_ops runs, the gpios listed in the
property are asserted/deasserted if the specified brightness is non-zero/zero.
Tested on a pxa270-based Palm Treo 680.
Signed-off-by: Mike Dunn miked...@newsguy.com
---
Thanks for looking!
.../bindings/video/backlight/pwm-backlight.txt | 4 +
drivers/video/backlight/pwm_bl.c | 128 ++---
2 files changed, 113 insertions(+), 19 deletions(-)
diff --git
a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..4583e68 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -14,6 +14,9 @@ Required properties:
Optional properties:
- pwm-names: a list of names for the PWM devices specified in the
pwms property (see PWM binding[0])
+ - gpios: An arbitrary number of gpios that must be asserted when the
+ backlight is on, and de-asserted when off. They will be
asserted
+ in the order they appear, and de-asserted in reverse order.
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
@@ -25,4 +28,5 @@ Example:
brightness-levels = 0 4 8 16 32 64 128 255;
default-brightness-level = 6;
+ gpios = gpio 77 0, gpio 25 1; /* gpio 25 is active low */
};
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 1fea627..1e2ab52 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -20,6 +20,12 @@
#include linux/pwm.h
#include linux/pwm_backlight.h
#include linux/slab.h
+#include linux/of_gpio.h
+
+struct pwm_bl_gpio {
+ unsigned int gpio;
+ enum of_gpio_flags flags;
+};
struct pwm_bl_data {
struct pwm_device *pwm;
@@ -27,6 +33,8 @@ struct pwm_bl_data {
unsigned intperiod;
unsigned intlth_brightness;
unsigned int*levels;
+ unsigned intnum_gpios;
+ struct pwm_bl_gpio *gpios;
int (*notify)(struct device *,
int brightness);
void(*notify_after)(struct device *,
@@ -94,14 +102,77 @@ static const struct backlight_ops pwm_backlight_ops = {
};
#ifdef CONFIG_OF
+static int pwm_backlight_dt_notify(struct device *dev, int brightness)
+{
+ struct backlight_device *bl = dev_get_drvdata(dev);
+ struct pwm_bl_data *pb = bl_get_data(bl);
+ int i;
+
+ if (brightness) {
+ for (i = 0; i pb-num_gpios; i++) {
+ if (pb-gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb-gpios[i].gpio, 0);
+ else
+ gpio_set_value(pb-gpios[i].gpio, 1);
+ }
+ return 0;
+ }
+
+ /* de-assert gpios in reverse order, in case this is important */
+ for (i = pb-num_gpios - 1; i = 0; i--) {
+ if (pb-gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ gpio_set_value(pb-gpios[i].gpio, 1);
+ else
+ gpio_set_value(pb-gpios[i].gpio, 0);
+ }
+ return 0;
+}
+
+static void pwm_backlight_dt_exit(struct pwm_bl_data *pb)
+{
+ int i;
+
+ for (i = 0; i pb-num_gpios; i++)
+ gpio_free(pb-gpios[i].gpio);
+}
+
+static int pwm_backlight_dt_init(struct device *dev, struct pwm_bl_data *pb)
+{
+ int i, j, ret;
+
+ /* request gpios and drive in the inactive state */
+ for (i = 0; i pb-num_gpios; i++) {
+ char gpio_name[32];
+ unsigned long flags;
+ if (pb-gpios[i].flags == OF_GPIO_ACTIVE_LOW)
+ flags = GPIOF_OUT_INIT_LOW;
+ else
+ flags = GPIOF_OUT_INIT_HIGH;
+ snprintf(gpio_name, 32, %s.%d, dev_name(dev), i);
+ ret = gpio_request_one(pb-gpios[i].gpio, flags, gpio_name);
+ if (ret) {
+ dev_err(dev, gpio #%d request failed\n, i);
+ goto gpio_err;
+ }
+ }
+ return 0;
+
+ gpio_err:
+ for (j = 0; j i; j++)
+ gpio_free(pb-gpios[j].gpio);
+ return ret;
+}
+
static int pwm_backlight_parse_dt(struct device *dev,
- struct
