| Message ID | 1309430517-23821-2-git-send-email-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sascha: I'm completely against this effort. The objections to my submission weren't ever because I was changing the user-visible API, so I don't think you can claim any advantage to mine in that regard. I will take some blame for not getting my API finished, but I have been fighting some serious non-work issues that have consumed nearly all of my available time--- including the professional time I would otherwise have to invest in getting my code finished. Is there the possibility that we could cooperate to get my patches finished, rather than discarding and reinventing them completely? b.g.
On Thursday 30 June 2011, Bill Gatliff wrote: > I'm completely against this effort. > > The objections to my submission weren't ever because I was changing > the user-visible API, so I don't think you can claim any advantage to > mine in that regard. > > I will take some blame for not getting my API finished, but I have > been fighting some serious non-work issues that have consumed nearly > all of my available time--- including the professional time I would > otherwise have to invest in getting my code finished. > > Is there the possibility that we could cooperate to get my patches > finished, rather than discarding and reinventing them completely? I've looked at your patches again, and it seems that you are doing two distinct changes, both good: 1. You provide a generic framework for pwm drivers that makes it possible for multiple drivers to coexist and simplifies the way that these drivers interact with the core OS. 2. You extend and fix a number of aspects in the global PWM API. Sascha's patch does only part 1, not part 2, but I don't think that makes his patches any worse. The introduction of the framework now is very similar to what you had suggested, and you should probably be mentioned in the changelog, even though the two implementations were done independently. A lot of people want to see a framework get merged, and I think it's great that Sascha has volunteered to do the work to push that through this time, especially since you have not been able to finish your work. What I think would be the best plan forward is to merge Sascha's patches as soon as we can, then get all currently existing pwm drivers converted to that and moved to drivers/pwm, and finally do the interface changes that you have proposed earlier. I would also hope that you can give constructive feedback to the submission and point out potential problems that you see where the code should be changed now in order to make your interface changes more easy later. I realize that it's annoying to spend a lot of time on a specific implementation and then see competing code get merged. Unfortunately, this happens all the time, and the code we merge is often not the one that has had the most effort spent on it, but the one that looks most promising at the time when it gets merged. Arnd
Guys: On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann <arnd@arndb.de> wrote: > A lot of people want to see a framework get merged, and I think it's > great that Sascha has volunteered to do the work to push that > through this time, especially since you have not been able to > finish your work. Sascha is wasting his time by reinventing the wheel. He's traveling over exactly the same path I have already covered. In fact, some of his reviewer comments are almost word-for-word the same as those I have received and addressed in the past. My patches were always kept current in this mailing list and others, and Sascha clearly has the skills necessary to make improvements and corrections should he have chosen to do so. > What I think would be the best plan forward is to merge Sascha's > patches as soon as we can, then get all currently existing pwm > drivers converted to that and moved to drivers/pwm, and finally > do the interface changes that you have proposed earlier. That's a real duplication of work. That makes no sense. > I would also hope that you can give constructive feedback to > the submission and point out potential problems that you see > where the code should be changed now in order to make your > interface changes more easy later. My code has already moved past that point. And if I had the time to evaluate and improve Sascha's patches, I would have the time to finish my own. Sascha is more than welcome to apply his efforts to the preexisting PWM API patches already present in the LKML archives. > I realize that it's annoying to spend a lot of time on a specific > implementation and then see competing code get merged. Annoyed isn't the word you are looking for. My code has been reviewed, tested and actively used as posted in LKML by several reviewers (including myself) in actual hardware. We are consciously choosing to discard a known entity and restart the whole process with new code. That's a waste of everyone's time and risk exposure. I don't consider Sascha's code to be "competing" with mine, as apparently nobody has bothered to consider one against the other. > Unfortunately, > this happens all the time, and the code we merge is often not > the one that has had the most effort spent on it, but the one that > looks most promising at the time when it gets merged. Your definition of "promsing" apparently correlates with "new and shiny". b.g.
Bill, On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: > Guys: > > On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > > A lot of people want to see a framework get merged, and I think it's > > great that Sascha has volunteered to do the work to push that > > through this time, especially since you have not been able to > > finish your work. > > Sascha is wasting his time by reinventing the wheel. He's traveling > over exactly the same path I have already covered. In fact, some of > his reviewer comments are almost word-for-word the same as those I > have received and addressed in the past. > > My patches were always kept current in this mailing list and others, > and Sascha clearly has the skills necessary to make improvements and > corrections should he have chosen to do so. I think that you made the fundamental mistake to completly ignore the existing pwm API and its users. With a competing api we are basically stuck. We can't convert the existing hardware drivers to the new API because leds-pwm.c, pwm_bl.c and others still depend on the old API and boards using it would break. We can't convert the function drivers either because again this would break boards for which only an old style pwm driver exists. So the logical thing to do is to put a step in between: Consolidate the existing drivers and *then* change the API atomically so that nothing breaks. Your patches don't do this, so I don't think at all that what I did is duplication of work. Given the current rush to move drivers out of arch/ it probably won't take long until all pwm drivers are moved to drivers/pwm/ and converted to use the framework, and then you have a good base to put your work onto. So please don't complain too much: We are currently only doing the work you didn't want to do. Sascha
Sascha: On Thu, Jun 30, 2011 at 12:02 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > So please don't complain too much: We are currently only doing the work > you didn't want to do. Per request of some the early reviewers, my plan was to migrate existing users over to the new API after the API itself was committed to mainstream. At any rate, this isn't a question of work I didn't "want" to do. On the one hand, I was simply doing what was asked of me. On the other hand, I was trying to make progress while simultaneously dealing with serious family matters that are still unresolved. At any rate, it's clear that your patches are going in, and mine aren't--- I have no choice but to accept that. I'm just frustrated by the duplication of effort. b.g.
On 01/07/11 03:02, Sascha Hauer wrote: > Bill, > > On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: >> Guys: >> >> On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann<arnd@arndb.de> wrote: >> >>> A lot of people want to see a framework get merged, and I think it's >>> great that Sascha has volunteered to do the work to push that >>> through this time, especially since you have not been able to >>> finish your work. >> Sascha is wasting his time by reinventing the wheel. He's traveling >> over exactly the same path I have already covered. In fact, some of >> his reviewer comments are almost word-for-word the same as those I >> have received and addressed in the past. >> >> My patches were always kept current in this mailing list and others, >> and Sascha clearly has the skills necessary to make improvements and >> corrections should he have chosen to do so. > I think that you made the fundamental mistake to completly ignore the > existing pwm API and its users. With a competing api we are basically > stuck. We can't convert the existing hardware drivers to the new API > because leds-pwm.c, pwm_bl.c and others still depend on the old API and > boards using it would break. I don't think this is really a blocker to Bill's patches. There are three (that I can see) pwm users currently: drivers/video/backlight/pwm_bl.c drivers/leds/leds-pwm.c drivers/input/misc/pwm-beeper.c All of those drivers are trivial and good easily be updated to work with Bill's patches. Bill already provided a leds-pwm driver. There is also the interesting case of the Atmel pwm driver, which does not fit the current pwm api and has its own backlight and leds drivers. Bill's patches addressed this, Sasha's patches do not. If we merge Sasha's patches then we are going to be in the same position as Bill's patches at some point in that we need to change the pwm api (and all its users) to meet the needs of the Atmel (and any similar hardware) pwm device. The ep93xx pwm driver (drivers/misc/ep93xx-pwm.c) also does not fit the current api (though it could), but instead provides a sysfs interface to user-space. Again, this was addressed by Bill's patches. > We can't convert the function drivers > either because again this would break boards for which only an old style > pwm driver exists. So the logical thing to do is to put a step in > between: Consolidate the existing drivers and *then* change the API > atomically so that nothing breaks. Your patches don't do this, so I > don't think at all that what I did is duplication of work. You have to modify the drivers anyway match the new pwm core. The Atmel and ep93xx drivers are going to be difficult to merge into the new api, and seeing as there are only about seven pwm drivers total in the kernel I think its a significant portion. Any pwm api patchset could easily convert all of the existing pwm drivers without becoming overly massive. If we merge Sasha's api, then we can move most of the existing drivers and maybe add some new ones, but we will still have the unconsolidated outliers. When (if?) we try to fix those we will probably need to change the pwm api and therefore all of the drivers to. So its basically a case of do the effort now (Bill's patches) or do it later. Doing it later will probably require more effort. > Given the current rush to move drivers out of arch/ it probably won't > take long until all pwm drivers are moved to drivers/pwm/ and converted > to use the framework, and then you have a good base to put your work onto. > So please don't complain too much: We are currently only doing the work > you didn't want to do. > You can move all of the drivers out of arch now if you want. You just need to make sure you are only compiling one of them in. The real job in consolidating means making sure that the api meets the needs of all of the drivers. The in kernel Atmel pwm driver at least is not going to convert easily to this api. Also, please not my change of email address for future emails. Thanks, ~Ryan
On Thursday, June 30, 2011 4:24 PM, Ryan Mallon wrote: > On 01/07/11 03:02, Sascha Hauer wrote: >> I think that you made the fundamental mistake to completly ignore the >> existing pwm API and its users. With a competing api we are basically >> stuck. We can't convert the existing hardware drivers to the new API >> because leds-pwm.c, pwm_bl.c and others still depend on the old API and >> boards using it would break. > > I don't think this is really a blocker to Bill's patches. There are > three (that I can see) pwm users currently: > drivers/video/backlight/pwm_bl.c > drivers/leds/leds-pwm.c > drivers/input/misc/pwm-beeper.c > > All of those drivers are trivial and good easily be updated to work with > Bill's patches. Bill already provided a leds-pwm driver. I agree with Ryan. The three drivers listed above appear to be the only pwm "user" drivers. They are all trivial and could be updated easily. The "core" drivers using the existing API appear to be: arch/arm/mach-vt8500/pwm.c arch/arm/plat-mxc/pwm.c arch/arm/plat-pxa/pwm.c arch/arm/plat-samsung/pwm.c arch/blackfin/kernel/pwm.c arch/mips/jz4740/pwm.c arch/unicore32/kernel/pwm.c drivers/misc/ab8500-pwm.c drivers/mfd/twl6030-pwm.c > There is also the interesting case of the Atmel pwm driver, which does > not fit the current pwm api and has its own backlight and leds drivers. > Bill's patches addressed this, Sasha's patches do not. If we merge > Sasha's patches then we are going to be in the same position as Bill's > patches at some point in that we need to change the pwm api (and all its > users) to meet the needs of the Atmel (and any similar hardware) pwm device. > > The ep93xx pwm driver (drivers/misc/ep93xx-pwm.c) also does not fit the > current api (though it could), but instead provides a sysfs interface to > user-space. Again, this was addressed by Bill's patches. I modified the ep93xx pwm driver previously to work with Bill's patches. The use for that driver required user-space control of the pwm. Bill's patches handle that. >> We can't convert the function drivers >> either because again this would break boards for which only an old style >> pwm driver exists. So the logical thing to do is to put a step in >> between: Consolidate the existing drivers and *then* change the API >> atomically so that nothing breaks. Your patches don't do this, so I >> don't think at all that what I did is duplication of work. > > You have to modify the drivers anyway match the new pwm core. The Atmel > and ep93xx drivers are going to be difficult to merge into the new api, > and seeing as there are only about seven pwm drivers total in the kernel > I think its a significant portion. Any pwm api patchset could easily > convert all of the existing pwm drivers without becoming overly massive. > > If we merge Sasha's api, then we can move most of the existing drivers > and maybe add some new ones, but we will still have the unconsolidated > outliers. When (if?) we try to fix those we will probably need to change > the pwm api and therefore all of the drivers to. So its basically a case > of do the effort now (Bill's patches) or do it later. Doing it later > will probably require more effort. > >> Given the current rush to move drivers out of arch/ it probably won't >> take long until all pwm drivers are moved to drivers/pwm/ and converted >> to use the framework, and then you have a good base to put your work onto. >> So please don't complain too much: We are currently only doing the work >> you didn't want to do. >> > You can move all of the drivers out of arch now if you want. You just > need to make sure you are only compiling one of them in. The real job in > consolidating means making sure that the api meets the needs of all of > the drivers. The in kernel Atmel pwm driver at least is not going to > convert easily to this api. I think the only open issue with Bill's patches was where the pwm subsystem belongs. Some people thought it should be part of gpiolib and others thought it should be on it's own. The core library Bill presented does share some commonality with gpiolib but I feel it still represents it's own subsystem. On-chip pwms might also be gpio capable but this is not always the case. With Bill's patches we could run into resource conflicts between gpio and pwm but that really exists now anyway. The pinmux subsystem that Linus Walleij (cc'ed) has been proposing should fix that. Overall I think Bill's patches will get us to the desired end result quicker and save a lot of needless churn. Regards, Hartley
On Thu, Jun 30, 2011 at 20:33, H Hartley Sweeten wrote: > On Thursday, June 30, 2011 4:24 PM, Ryan Mallon wrote: >> On 01/07/11 03:02, Sascha Hauer wrote: >>> I think that you made the fundamental mistake to completly ignore the >>> existing pwm API and its users. With a competing api we are basically >>> stuck. We can't convert the existing hardware drivers to the new API >>> because leds-pwm.c, pwm_bl.c and others still depend on the old API and >>> boards using it would break. >> >> I don't think this is really a blocker to Bill's patches. There are >> three (that I can see) pwm users currently: >> drivers/video/backlight/pwm_bl.c >> drivers/leds/leds-pwm.c >> drivers/input/misc/pwm-beeper.c >> >> All of those drivers are trivial and good easily be updated to work with >> Bill's patches. Bill already provided a leds-pwm driver. > > I agree with Ryan. The three drivers listed above appear to be the only > pwm "user" drivers. They are all trivial and could be updated easily. indeed ... the current pwm API is almost toy like in its simplicity. > The "core" drivers using the existing API appear to be: > arch/blackfin/kernel/pwm.c i only wrote this because Bill's subsystem wasnt yet merged. but i made sure that it was done in such a way that it's trivial to convert over to Bill's work once it's done. -mike
On Fri, Jul 01, 2011 at 09:24:05AM +1000, Ryan Mallon wrote: > On 01/07/11 03:02, Sascha Hauer wrote: > >Bill, > > > >On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: > >>Guys: > >> > >>On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann<arnd@arndb.de> wrote: > >> > >>>A lot of people want to see a framework get merged, and I think it's > >>>great that Sascha has volunteered to do the work to push that > >>>through this time, especially since you have not been able to > >>>finish your work. > >>Sascha is wasting his time by reinventing the wheel. He's traveling > >>over exactly the same path I have already covered. In fact, some of > >>his reviewer comments are almost word-for-word the same as those I > >>have received and addressed in the past. > >> > >>My patches were always kept current in this mailing list and others, > >>and Sascha clearly has the skills necessary to make improvements and > >>corrections should he have chosen to do so. > >I think that you made the fundamental mistake to completly ignore the > >existing pwm API and its users. With a competing api we are basically > >stuck. We can't convert the existing hardware drivers to the new API > >because leds-pwm.c, pwm_bl.c and others still depend on the old API and > >boards using it would break. > > I don't think this is really a blocker to Bill's patches. There are > three (that I can see) pwm users currently: > drivers/video/backlight/pwm_bl.c > drivers/leds/leds-pwm.c > drivers/input/misc/pwm-beeper.c > > All of those drivers are trivial and good easily be updated to work > with Bill's patches. Bill already provided a leds-pwm driver. Yes, it is easy but that's not my point. The point is that you can't convert the drivers without converting *all* hardware drivers in a *single* step. If you choose to have two competing APIs in the tree for one purpose you can't convert the drivers but instead you have to copy them, either with cp or ifdefs. I have just looked at the leds-pwm driver Bill provided. Applying it immediately breaks all users. > > There is also the interesting case of the Atmel pwm driver, which > does not fit the current pwm api and has its own backlight and leds > drivers. Bill's patches addressed this, Sasha's patches do not. If > we merge Sasha's patches then we are going to be in the same > position as Bill's patches at some point in that we need to change > the pwm api (and all its users) to meet the needs of the Atmel (and > any similar hardware) pwm device. My patches are compatible to the current (user-) API on purpose. It offers the possibility to convert each hardware driver independently of the others. Once we have a framework we can change the driver API independent of the user API. > > The ep93xx pwm driver (drivers/misc/ep93xx-pwm.c) also does not fit > the current api (though it could), but instead provides a sysfs > interface to user-space. Again, this was addressed by Bill's > patches. > > >We can't convert the function drivers > >either because again this would break boards for which only an old style > >pwm driver exists. So the logical thing to do is to put a step in > >between: Consolidate the existing drivers and *then* change the API > >atomically so that nothing breaks. Your patches don't do this, so I > >don't think at all that what I did is duplication of work. > > You have to modify the drivers anyway match the new pwm core. Yes. But changing the user API *and* the driver API in a single patch really is a bad idea. I don't say at all that the end result Bill is aiming at is bad because it isn't. We are not talking about the end result, but only the way to get there. And getting from a to b in bisectable small steps is a well established development model in the Kernel, or have I missed something? > The > Atmel and ep93xx drivers are going to be difficult to merge into the > new api, and seeing as there are only about seven pwm drivers total > in the kernel I think its a significant portion. Any pwm api > patchset could easily convert all of the existing pwm drivers > without becoming overly massive. > > If we merge Sasha's api, then we can move most of the existing > drivers and maybe add some new ones, but we will still have the > unconsolidated outliers. When (if?) we try to fix those we will > probably need to change the pwm api and therefore all of the drivers > to. So its basically a case of do the effort now (Bill's patches) or > do it later. Doing it later will probably require more effort. > > >Given the current rush to move drivers out of arch/ it probably won't > >take long until all pwm drivers are moved to drivers/pwm/ and converted > >to use the framework, and then you have a good base to put your work onto. > >So please don't complain too much: We are currently only doing the work > >you didn't want to do. > > > > You can move all of the drivers out of arch now if you want. You > just need to make sure you are only compiling one of them in. Yes, we can. But this does not solve the migration problem I try to describe in this and the previous mail. Let's face it: Bill is working on his PWM patches for three years now, still the merge of these patches is not in sight. Let's just split them into smaller parts which are easier to swallow. > The > real job in consolidating means making sure that the api meets the > needs of all of the drivers. The in kernel Atmel pwm driver at least > is not going to convert easily to this api. Then let's change the API. I have nothing against this. > > Also, please not my change of email address for future emails. ack Sascha
On 01/07/11 17:37, Sascha Hauer wrote: > On Fri, Jul 01, 2011 at 09:24:05AM +1000, Ryan Mallon wrote: >> On 01/07/11 03:02, Sascha Hauer wrote: >>> Bill, >>> >>> On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: >>>> Guys: >>>> >>>> On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann<arnd@arndb.de> wrote: >>>> >>>>> A lot of people want to see a framework get merged, and I think it's >>>>> great that Sascha has volunteered to do the work to push that >>>>> through this time, especially since you have not been able to >>>>> finish your work. >>>> Sascha is wasting his time by reinventing the wheel. He's traveling >>>> over exactly the same path I have already covered. In fact, some of >>>> his reviewer comments are almost word-for-word the same as those I >>>> have received and addressed in the past. >>>> >>>> My patches were always kept current in this mailing list and others, >>>> and Sascha clearly has the skills necessary to make improvements and >>>> corrections should he have chosen to do so. >>> I think that you made the fundamental mistake to completly ignore the >>> existing pwm API and its users. With a competing api we are basically >>> stuck. We can't convert the existing hardware drivers to the new API >>> because leds-pwm.c, pwm_bl.c and others still depend on the old API and >>> boards using it would break. >> I don't think this is really a blocker to Bill's patches. There are >> three (that I can see) pwm users currently: >> drivers/video/backlight/pwm_bl.c >> drivers/leds/leds-pwm.c >> drivers/input/misc/pwm-beeper.c >> >> All of those drivers are trivial and good easily be updated to work >> with Bill's patches. Bill already provided a leds-pwm driver. > Yes, it is easy but that's not my point. The point is that you can't > convert the drivers without converting *all* hardware drivers in a > *single* step. If you choose to have two competing APIs in the tree > for one purpose you can't convert the drivers but instead you have > to copy them, either with cp or ifdefs. I have just looked at the > leds-pwm driver Bill provided. Applying it immediately breaks all > users. > At _any_ point that you change the pwm api you will need to change all of the drivers. How do you plan to adapt the api in the future to support the oddball pwm drivers without having to change all of the drivers in one go? The number of pwm drivers and users is so small that it would be nice to see a patch series that introduces the new framework/api and converts all of the drivers over. That way we don't get left in an intermediate state with some drivers using the new framework and some using the old one. >> There is also the interesting case of the Atmel pwm driver, which >> does not fit the current pwm api and has its own backlight and leds >> drivers. Bill's patches addressed this, Sasha's patches do not. If >> we merge Sasha's patches then we are going to be in the same >> position as Bill's patches at some point in that we need to change >> the pwm api (and all its users) to meet the needs of the Atmel (and >> any similar hardware) pwm device. > My patches are compatible to the current (user-) API on purpose. It > offers the possibility to convert each hardware driver independently > of the others. Once we have a framework we can change the driver API > independent of the user API. We should just convert all of the drivers now rather than waiting for the various maintainers to do it. Because your proposed api is backwards compatible and most SoC platforms don't have additional external pwms there is little motivation for them to do this work. >> The ep93xx pwm driver (drivers/misc/ep93xx-pwm.c) also does not fit >> the current api (though it could), but instead provides a sysfs >> interface to user-space. Again, this was addressed by Bill's >> patches. >> >>> We can't convert the function drivers >>> either because again this would break boards for which only an old style >>> pwm driver exists. So the logical thing to do is to put a step in >>> between: Consolidate the existing drivers and *then* change the API >>> atomically so that nothing breaks. Your patches don't do this, so I >>> don't think at all that what I did is duplication of work. >> You have to modify the drivers anyway match the new pwm core. > Yes. But changing the user API *and* the driver API in a single patch > really is a bad idea. It doesn't have to be in a single patch, but it should be in a single patch series. > I don't say at all that the end result Bill is aiming at is bad because > it isn't. We are not talking about the end result, but only the way to > get there. And getting from a to b in bisectable small steps is a well > established development model in the Kernel, or have I missed something? The bi-sectable steps is fine. But what we have is a patch series which introduces a new framework with a vague promise that the existing drivers will get converted and the api will be improved later. Again, the number of in-kernel users is so small that we may as well do it all in one series. >> The >> Atmel and ep93xx drivers are going to be difficult to merge into the >> new api, and seeing as there are only about seven pwm drivers total >> in the kernel I think its a significant portion. Any pwm api >> patchset could easily convert all of the existing pwm drivers >> without becoming overly massive. >> >> If we merge Sasha's api, then we can move most of the existing >> drivers and maybe add some new ones, but we will still have the >> unconsolidated outliers. When (if?) we try to fix those we will >> probably need to change the pwm api and therefore all of the drivers >> to. So its basically a case of do the effort now (Bill's patches) or >> do it later. Doing it later will probably require more effort. >> >>> Given the current rush to move drivers out of arch/ it probably won't >>> take long until all pwm drivers are moved to drivers/pwm/ and converted >>> to use the framework, and then you have a good base to put your work onto. >>> So please don't complain too much: We are currently only doing the work >>> you didn't want to do. >>> >> You can move all of the drivers out of arch now if you want. You >> just need to make sure you are only compiling one of them in. > Yes, we can. But this does not solve the migration problem I try > to describe in this and the previous mail. > > Let's face it: Bill is working on his PWM patches for three years now, > still the merge of these patches is not in sight. Let's just split > them into smaller parts which are easier to swallow. > Sure, but your series still leaves a lot of work to be done. How many years will the ep93xx and atmel pwm drivers exist in isolation from the rest of the pwm drivers? ~Ryan
On Fri, Jul 01, 2011 at 06:28:48PM +1000, Ryan Mallon wrote: > On 01/07/11 17:37, Sascha Hauer wrote: > > On Fri, Jul 01, 2011 at 09:24:05AM +1000, Ryan Mallon wrote: > >> On 01/07/11 03:02, Sascha Hauer wrote: > >>> Bill, > >>> > >>> On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: > >>>> Guys: > >>>> > >>>> On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann<arnd@arndb.de> wrote: > >>>> > >>>>> A lot of people want to see a framework get merged, and I think it's > >>>>> great that Sascha has volunteered to do the work to push that > >>>>> through this time, especially since you have not been able to > >>>>> finish your work. > >>>> Sascha is wasting his time by reinventing the wheel. He's traveling > >>>> over exactly the same path I have already covered. In fact, some of > >>>> his reviewer comments are almost word-for-word the same as those I > >>>> have received and addressed in the past. > >>>> > >>>> My patches were always kept current in this mailing list and others, > >>>> and Sascha clearly has the skills necessary to make improvements and > >>>> corrections should he have chosen to do so. > >>> I think that you made the fundamental mistake to completly ignore the > >>> existing pwm API and its users. With a competing api we are basically > >>> stuck. We can't convert the existing hardware drivers to the new API > >>> because leds-pwm.c, pwm_bl.c and others still depend on the old API and > >>> boards using it would break. > >> I don't think this is really a blocker to Bill's patches. There are > >> three (that I can see) pwm users currently: > >> drivers/video/backlight/pwm_bl.c > >> drivers/leds/leds-pwm.c > >> drivers/input/misc/pwm-beeper.c > >> > >> All of those drivers are trivial and good easily be updated to work > >> with Bill's patches. Bill already provided a leds-pwm driver. > > Yes, it is easy but that's not my point. The point is that you can't > > convert the drivers without converting *all* hardware drivers in a > > *single* step. If you choose to have two competing APIs in the tree > > for one purpose you can't convert the drivers but instead you have > > to copy them, either with cp or ifdefs. I have just looked at the > > leds-pwm driver Bill provided. Applying it immediately breaks all > > users. > > > > At _any_ point that you change the pwm api you will need to change all > of the drivers. How do you plan to adapt the api in the future to > support the oddball pwm drivers without having to change all of the > drivers in one go? It is a framework between hardware drivers and user drivers and as such it is able to abstract between the API to the hardware drivers and the API to the user drivers. You can change one of those without touching the other. > > The number of pwm drivers and users is so small that it would be nice to > see a patch series that introduces the new framework/api and converts > all of the drivers over. That way we don't get left in an intermediate > state with some drivers using the new framework and some using the old one. Go ahead, send patches. > > >> There is also the interesting case of the Atmel pwm driver, which > >> does not fit the current pwm api and has its own backlight and leds > >> drivers. Bill's patches addressed this, Sasha's patches do not. If > >> we merge Sasha's patches then we are going to be in the same > >> position as Bill's patches at some point in that we need to change > >> the pwm api (and all its users) to meet the needs of the Atmel (and > >> any similar hardware) pwm device. > > My patches are compatible to the current (user-) API on purpose. It > > offers the possibility to convert each hardware driver independently > > of the others. Once we have a framework we can change the driver API > > independent of the user API. > > We should just convert all of the drivers now rather than waiting for > the various maintainers to do it. Because your proposed api is backwards > compatible and most SoC platforms don't have additional external pwms > there is little motivation for them to do this work. And with a compatible API this is an easy job. Just remove the list handling from the drivers and put the pwm_* functions into callbacks. The motivation to do this comes from the people who want to change the pwm API. I volunteer to convert at least the PXA and i.MX driver. > > >> The ep93xx pwm driver (drivers/misc/ep93xx-pwm.c) also does not fit > >> the current api (though it could), but instead provides a sysfs > >> interface to user-space. Again, this was addressed by Bill's > >> patches. > >> > >>> We can't convert the function drivers > >>> either because again this would break boards for which only an old style > >>> pwm driver exists. So the logical thing to do is to put a step in > >>> between: Consolidate the existing drivers and *then* change the API > >>> atomically so that nothing breaks. Your patches don't do this, so I > >>> don't think at all that what I did is duplication of work. > >> You have to modify the drivers anyway match the new pwm core. > > Yes. But changing the user API *and* the driver API in a single patch > > really is a bad idea. > > It doesn't have to be in a single patch, but it should be in a single > patch series. > > > I don't say at all that the end result Bill is aiming at is bad because > > it isn't. We are not talking about the end result, but only the way to > > get there. And getting from a to b in bisectable small steps is a well > > established development model in the Kernel, or have I missed something? > > The bi-sectable steps is fine. But what we have is a patch series which > introduces a new framework with a vague promise that the existing > drivers will get converted and the api will be improved later. Again, > the number of in-kernel users is so small that we may as well do it all > in one series. I don't understand you. On one hand you argue that it's trivial enough to port all drivers over the a new API in a single series and on the other hand you argue that we have to wait for several subsystem maintainers to port the drivers over to an existing-but-now-in-a-framework API. > > >> The > >> Atmel and ep93xx drivers are going to be difficult to merge into the > >> new api, and seeing as there are only about seven pwm drivers total > >> in the kernel I think its a significant portion. Any pwm api > >> patchset could easily convert all of the existing pwm drivers > >> without becoming overly massive. > >> > >> If we merge Sasha's api, then we can move most of the existing > >> drivers and maybe add some new ones, but we will still have the > >> unconsolidated outliers. When (if?) we try to fix those we will > >> probably need to change the pwm api and therefore all of the drivers > >> to. So its basically a case of do the effort now (Bill's patches) or > >> do it later. Doing it later will probably require more effort. > >> > >>> Given the current rush to move drivers out of arch/ it probably won't > >>> take long until all pwm drivers are moved to drivers/pwm/ and converted > >>> to use the framework, and then you have a good base to put your work onto. > >>> So please don't complain too much: We are currently only doing the work > >>> you didn't want to do. > >>> > >> You can move all of the drivers out of arch now if you want. You > >> just need to make sure you are only compiling one of them in. > > Yes, we can. But this does not solve the migration problem I try > > to describe in this and the previous mail. > > > > Let's face it: Bill is working on his PWM patches for three years now, > > still the merge of these patches is not in sight. Let's just split > > them into smaller parts which are easier to swallow. > > > > Sure, but your series still leaves a lot of work to be done. How many > years will the ep93xx and atmel pwm drivers exist in isolation from the > rest of the pwm drivers? If we continue to argue probably many years... Sascha
On 01/07/11 18:54, Sascha Hauer wrote: > On Fri, Jul 01, 2011 at 06:28:48PM +1000, Ryan Mallon wrote: >> On 01/07/11 17:37, Sascha Hauer wrote: >>> On Fri, Jul 01, 2011 at 09:24:05AM +1000, Ryan Mallon wrote: >>>> On 01/07/11 03:02, Sascha Hauer wrote: >>>>> Bill, >>>>> >>>>> On Thu, Jun 30, 2011 at 11:17:54AM -0500, Bill Gatliff wrote: >>>>>> Guys: >>>>>> >>>>>> On Thu, Jun 30, 2011 at 7:41 AM, Arnd Bergmann<arnd@arndb.de> wrote: >>>>>> >>>>>>> A lot of people want to see a framework get merged, and I think it's >>>>>>> great that Sascha has volunteered to do the work to push that >>>>>>> through this time, especially since you have not been able to >>>>>>> finish your work. >>>>>> Sascha is wasting his time by reinventing the wheel. He's traveling >>>>>> over exactly the same path I have already covered. In fact, some of >>>>>> his reviewer comments are almost word-for-word the same as those I >>>>>> have received and addressed in the past. >>>>>> >>>>>> My patches were always kept current in this mailing list and others, >>>>>> and Sascha clearly has the skills necessary to make improvements and >>>>>> corrections should he have chosen to do so. >>>>> I think that you made the fundamental mistake to completly ignore the >>>>> existing pwm API and its users. With a competing api we are basically >>>>> stuck. We can't convert the existing hardware drivers to the new API >>>>> because leds-pwm.c, pwm_bl.c and others still depend on the old API and >>>>> boards using it would break. >>>> I don't think this is really a blocker to Bill's patches. There are >>>> three (that I can see) pwm users currently: >>>> drivers/video/backlight/pwm_bl.c >>>> drivers/leds/leds-pwm.c >>>> drivers/input/misc/pwm-beeper.c >>>> >>>> All of those drivers are trivial and good easily be updated to work >>>> with Bill's patches. Bill already provided a leds-pwm driver. >>> Yes, it is easy but that's not my point. The point is that you can't >>> convert the drivers without converting *all* hardware drivers in a >>> *single* step. If you choose to have two competing APIs in the tree >>> for one purpose you can't convert the drivers but instead you have >>> to copy them, either with cp or ifdefs. I have just looked at the >>> leds-pwm driver Bill provided. Applying it immediately breaks all >>> users. >>> >> At _any_ point that you change the pwm api you will need to change all >> of the drivers. How do you plan to adapt the api in the future to >> support the oddball pwm drivers without having to change all of the >> drivers in one go? > It is a framework between hardware drivers and user drivers and as such > it is able to abstract between the API to the hardware drivers and the > API to the user drivers. You can change one of those without touching > the other. You are missing the point. For example, the current in-kernel api to configure a pwm is this: int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); Bill's proposed change to the api, which is more flexible, looks like this: int pwm_config(struct pwm_channel *p, struct pwm_channel_config *c) If you want to change the api in this way at any time, you will need to change all of the pwm drivers and all of the users (pwm-bl, pwm-leds, etc) at the same time, or introduce some sort of wrapper layer to ease the conversion. Either way, if you do it now or do it after your patch set it is still the same amount of work. We do want a better api for pwm drivers, if only to support oddball hardware like the atmel. The process I see is this if we go with Bill's patches: - Introduce new api with common framework - Move drivers to new api With your patches we get this: - Introduce common framework - Move drivers to common framework - Modify api - Rewrite drivers for new api The problem we have seen with these processes in the past is they get left half way with some drivers moving to the new framework and some trailing behind and some having their own hand-rolled approach because the common api doesn't meet their needs. >>> I don't say at all that the end result Bill is aiming at is bad because >>> it isn't. We are not talking about the end result, but only the way to >>> get there. And getting from a to b in bisectable small steps is a well >>> established development model in the Kernel, or have I missed something? >> The bi-sectable steps is fine. But what we have is a patch series which >> introduces a new framework with a vague promise that the existing >> drivers will get converted and the api will be improved later. Again, >> the number of in-kernel users is so small that we may as well do it all >> in one series. > I don't understand you. On one hand you argue that it's trivial enough to > port all drivers over the a new API in a single series and on the other > hand you argue that we have to wait for several subsystem maintainers > to port the drivers over to an existing-but-now-in-a-framework API. > No, I am saying that if we don't port all the drivers and leave it up to the maintainers then it won't get done and we get left with some drivers using the old approach and some using the new approach. For example, we still have platforms which aren't using gpiolib yet and that was introduced 3 years ago. ~Ryan
On Sat, Jul 02, 2011 at 10:40:54AM +1000, Ryan Mallon wrote: > On 01/07/11 18:54, Sascha Hauer wrote: > >> At _any_ point that you change the pwm api you will need to change all > >> of the drivers. How do you plan to adapt the api in the future to > >> support the oddball pwm drivers without having to change all of the > >> drivers in one go? > > It is a framework between hardware drivers and user drivers and as such > > it is able to abstract between the API to the hardware drivers and the > > API to the user drivers. You can change one of those without touching > > the other. > > You are missing the point. For example, the current in-kernel api to > configure a pwm is this: > > int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > > Bill's proposed change to the api, which is more flexible, looks like this: > > int pwm_config(struct pwm_channel *p, > struct pwm_channel_config *c) > > If you want to change the api in this way at any time, you will need to > change all of the pwm drivers and all of the users (pwm-bl, pwm-leds, > etc) at the same time, or introduce some sort of wrapper layer to ease > the conversion. Either way, if you do it now or do it after your patch > set it is still the same amount of work. > > We do want a better api for pwm drivers, if only to support oddball > hardware like the atmel. The process I see is this if we go with Bill's > patches: > - Introduce new api with common framework I'm repeating myself. At this point you can't convert any of the old drivers because it breaks the support we already have. > - Move drivers to new api And that's one single step. To be really bisectible have would have to convert *all* PWM drivers in a single patch, not even a patch series as it would break bisectibility in between. > > With your patches we get this: > - Introduce common framework And still everything works > - Move drivers to common framework One after another, still everything works > - Modify api > - Rewrite drivers for new api There are two of them. One for the hardware drivers and one for the users. You can change them independently, introduce wrappers where necessary, you have all the time in the world to do this. Your aim is to change the API? Then you have to clean up first. People like Thomas have cleaned up in all areas of the kernel to reach their goal of merging the RT stuff. They wouldn't have got anywhere with a here's-a-patch-that-changes-everything approach. > > The problem we have seen with these processes in the past is they get > left half way with some drivers moving to the new framework and some > trailing behind and some having their own hand-rolled approach because > the common api doesn't meet their needs. With two competing APIs it's even worse because you can't go anywhere without converting everything in tree. Just imagine Bill or someone else comes up with a series PATCH: introduce new pwm framework with completely changed API, sysfs interface and interrupt support, also convert leds-pwm, pwm_bl, pwm-beeper to new API and convert all hardware drivers to new API aswell. What do you think would happen? I can think of two things: Either no reaction at all from others or a huge thread with no result at all because we can't agree on details. Then someone comes up with the recommendation to start small and do it in little steps. I am tired of discussing this. It seems we can't agree and unless someone else jumps in here we will probably have to wait for another year until something moves in the PWM area. Sascha
Sascha Hauer wrote: > I am tired of discussing this. It seems we can't agree and unless > someone else jumps in here we will probably have to wait for another > year until something moves in the PWM area. Please continue your work on PWM area! Patch reviews show a really positive attitute towards merging your patchset. Please don't stop here!
On Monday 04 July 2011, Kurt Van Dijck wrote: > > > > The pwm framework needs to incorporate at least the following: > > - sysfs access (ep93xx driver) > > - Multiple channels per device (atmel driver) > > These are 2 very hardware dependant additions. Is this really the job > for a framework to incorporate this? > IMHO, the job of a framework is to allow such things. Creating a framework > that does all special things of all vendors makes such thing > complicated. I think you shouldn't make it too easy for drivers to add extra sysfs files. If at all possible, the default should be to add them to the core, and define them in a way that makes sense for future similar drivers. We need to be careful to avoid a situation where multiple driver writers introduce the same feature with a sysfs attribute, but do so in an incompatible way. > With socketCAN, we encountered a similar problem. Every chip maker > tries to create added value by means of special options. You can't > support them all in the framework. Therefore, sysfs can be added > to configure special things. I would expect that pwm is much simpler than CAN, so the amount of creativity there is also limited. Arnd
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt new file mode 100644 index 0000000..c7c5cb1 --- /dev/null +++ b/Documentation/pwm.txt @@ -0,0 +1,56 @@ +Pulse Width Modulation (PWM) interface + +This provides an overview about the Linux PWM interface + +PWMs are commonly used for controlling LEDs, fans or vibrators in +cell phones. PWMs with a fixed purpose have no need implementing +the Linux PWM API (although they could). However, PWMs are often +found as discrete devices on SoCs which have no fixed purpose. It's +up to the board designer to connect them to LEDs or fans. To provide +this kind of flexibility the generic PWM API exists. + +Identifying PWMs +---------------- + +PWMs are identified by unique ids throughout the system. A platform +should call pwmchip_reserve() during init time to reserve the id range +for internal PWMs so that users have a fixed id to refer to specific +PWMs. + +Using PWMs +---------- + +A PWM can be requested using pwm_request() and freed after usage with +pwm_free(). After being requested a PWM has to be configured using + +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); + +To start/stop toggling the PWM output use pwm_enable()/pwm_disable(). + +Implementing a PWM driver +------------------------- + +Currently there are two ways to implement pwm drivers. Traditionally +there only has been the barebone API meaning that each driver has +to implement the pwm_*() functions itself. This means that it's impossible +to have multiple PWM drivers in the system. For this reason it's mandatory +for new drivers to use the generic PWM framework. +A new PWM device can be added using pwmchip_add() and removed again with +pwmchip_remove(). pwmchip_add() takes a filled in struct pwm_chip as +argument which provides the ops and the pwm id to the framework. + +Locking +------- + +The PWM core list manipulations are protected by a mutex, so pwm_request() +and pwm_free() may not be called from an atomic context. Currently the +PWM core does not enforce any locking to pwm_enable(), pwm_disable() and +pwm_config(), so the calling context is currently driver specific. This +is an issue derived from the former barebone API and should be fixed soon. + +Helpers +------- + +Currently a PWM can only be configured with period_ns and duty_ns. For several +use cases freq_hz and duty_percent might be better. Instead of calculating +this in your driver please consider adding appropriate helpers to the framework. diff --git a/MAINTAINERS b/MAINTAINERS index f0358cd..79c047b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5142,6 +5142,12 @@ M: Robert Jarzmik <robert.jarzmik@free.fr> L: rtc-linux@googlegroups.com S: Maintained +PWM core +M: Sascha Hauer <s.hauer@pengutronix.de> +L: linux-kernel@vger.kernel.org +S: Maintained +F: drivers/pwm/ + QLOGIC QLA1280 SCSI DRIVER M: Michael Reed <mdr@sgi.com> L: linux-scsi@vger.kernel.org diff --git a/drivers/Kconfig b/drivers/Kconfig index 3bb154d..67f5c27 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -126,4 +126,6 @@ source "drivers/hwspinlock/Kconfig" source "drivers/clocksource/Kconfig" +source "drivers/pwm/Kconfig" + endmenu diff --git a/drivers/Makefile b/drivers/Makefile index 09f3232..c321763 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -6,6 +6,7 @@ # obj-y += gpio/ +obj-y += pwm/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ obj-$(CONFIG_RAPIDIO) += rapidio/ diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig new file mode 100644 index 0000000..93c1052 --- /dev/null +++ b/drivers/pwm/Kconfig @@ -0,0 +1,12 @@ +menuconfig PWM + bool "PWM Support" + help + This enables PWM support through the generic PWM framework. + You only need to enable this, if you also want to enable + one or more of the PWM drivers below. + + If unsure, say N. + +if PWM + +endif diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile new file mode 100644 index 0000000..3469c3d --- /dev/null +++ b/drivers/pwm/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_PWM) += core.o diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c new file mode 100644 index 0000000..71de479 --- /dev/null +++ b/drivers/pwm/core.c @@ -0,0 +1,220 @@ +/* + * Generic pwmlib implementation + * + * Copyright (C) 2011 Sascha Hauer <s.hauer@pengutronix.de> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; see the file COPYING. If not, write to + * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. + */ +#include <linux/module.h> +#include <linux/pwm.h> +#include <linux/list.h> +#include <linux/mutex.h> +#include <linux/err.h> +#include <linux/slab.h> +#include <linux/device.h> + +struct pwm_device { + struct pwm_chip *chip; + const char *label; + unsigned long flags; +#define FLAG_REQUESTED 0 +#define FLAG_ENABLED 1 + struct list_head node; +}; + +static LIST_HEAD(pwm_list); + +static DEFINE_MUTEX(pwm_lock); + +static struct pwm_device *_find_pwm(int pwm_id) +{ + struct pwm_device *pwm; + + list_for_each_entry(pwm, &pwm_list, node) { + if (pwm->chip->pwm_id == pwm_id) + return pwm; + } + + return NULL; +} + +/** + * pwmchip_add() - register a new pwm + * @chip: the pwm + * + * register a new pwm. pwm->pwm_id must be initialized. if pwm_id < 0 then + * a dynamically assigned id will be used, otherwise the id specified, + */ +int pwmchip_add(struct pwm_chip *chip) +{ + struct pwm_device *pwm; + int ret = 0; + + pwm = kzalloc(sizeof(*pwm), GFP_KERNEL); + if (!pwm) + return -ENOMEM; + + pwm->chip = chip; + + mutex_lock(&pwm_lock); + + if (chip->pwm_id >= 0 && _find_pwm(chip->pwm_id)) { + ret = -EBUSY; + goto out; + } + + list_add_tail(&pwm->node, &pwm_list); +out: + mutex_unlock(&pwm_lock); + + if (ret) + kfree(pwm); + + return ret; +} +EXPORT_SYMBOL_GPL(pwmchip_add); + +/** + * pwmchip_remove() - remove a pwm + * @chip: the pwm + * + * remove a pwm. This function may return busy if the pwm is still requested. + */ +int pwmchip_remove(struct pwm_chip *chip) +{ + struct pwm_device *pwm; + int ret = 0; + + mutex_lock(&pwm_lock); + + pwm = _find_pwm(chip->pwm_id); + if (!pwm) { + ret = -ENOENT; + goto out; + } + + if (test_bit(FLAG_REQUESTED, &pwm->flags)) { + ret = -EBUSY; + goto out; + } + + list_del(&pwm->node); + + kfree(pwm); +out: + mutex_unlock(&pwm_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(pwmchip_remove); + +/* + * pwm_request - request a PWM device + */ +struct pwm_device *pwm_request(int pwm_id, const char *label) +{ + struct pwm_device *pwm; + int ret; + + mutex_lock(&pwm_lock); + + pwm = _find_pwm(pwm_id); + if (!pwm) { + pwm = ERR_PTR(-ENOENT); + goto out; + } + + if (test_bit(FLAG_REQUESTED, &pwm->flags)) { + pwm = ERR_PTR(-EBUSY); + goto out; + } + + if (!try_module_get(pwm->chip->ops->owner)) { + pwm = ERR_PTR(-ENODEV); + goto out; + } + + if (pwm->chip->ops->request) { + ret = pwm->chip->ops->request(pwm->chip); + if (ret) { + pwm = ERR_PTR(ret); + goto out_put; + } + } + + pwm->label = label; + set_bit(FLAG_REQUESTED, &pwm->flags); + + goto out; + +out_put: + module_put(pwm->chip->ops->owner); +out: + mutex_unlock(&pwm_lock); + + return pwm; +} +EXPORT_SYMBOL_GPL(pwm_request); + +/* + * pwm_free - free a PWM device + */ +void pwm_free(struct pwm_device *pwm) +{ + mutex_lock(&pwm_lock); + + if (!test_and_clear_bit(FLAG_REQUESTED, &pwm->flags)) { + pr_warning("PWM device already freed\n"); + goto out; + } + + pwm->label = NULL; + + module_put(pwm->chip->ops->owner); +out: + mutex_unlock(&pwm_lock); +} +EXPORT_SYMBOL_GPL(pwm_free); + +/* + * pwm_config - change a PWM device configuration + */ +int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns) +{ + return pwm->chip->ops->config(pwm->chip, duty_ns, period_ns); +} +EXPORT_SYMBOL_GPL(pwm_config); + +/* + * pwm_enable - start a PWM output toggling + */ +int pwm_enable(struct pwm_device *pwm) +{ + if (!test_and_set_bit(FLAG_ENABLED, &pwm->flags)) + return pwm->chip->ops->enable(pwm->chip); + + return 0; +} +EXPORT_SYMBOL_GPL(pwm_enable); + +/* + * pwm_disable - stop a PWM output toggling + */ +void pwm_disable(struct pwm_device *pwm) +{ + if (test_and_clear_bit(FLAG_ENABLED, &pwm->flags)) + pwm->chip->ops->disable(pwm->chip); +} +EXPORT_SYMBOL_GPL(pwm_disable); diff --git a/include/linux/pwm.h b/include/linux/pwm.h index 7c77575..df9681b 100644 --- a/include/linux/pwm.h +++ b/include/linux/pwm.h @@ -28,4 +28,41 @@ int pwm_enable(struct pwm_device *pwm); */ void pwm_disable(struct pwm_device *pwm); +#ifdef CONFIG_PWM +struct pwm_chip; + +/** + * struct pwm_ops - PWM operations + * @request: optional hook for requesting a PWM + * @free: optional hook for freeing a PWM + * @config: configure duty cycles and period length for this PWM + * @enable: enable PWM output toggling + * @disable: disable PWM output toggling + */ +struct pwm_ops { + int (*request)(struct pwm_chip *chip); + void (*free)(struct pwm_chip *chip); + int (*config)(struct pwm_chip *chip, int duty_ns, + int period_ns); + int (*enable)(struct pwm_chip *chip); + void (*disable)(struct pwm_chip *chip); + struct module *owner; +}; + +/** + * struct pwm_chip - abstract a PWM + * @label: for diagnostics + * @owner: helps prevent removal of modules exporting active PWMs + * @ops: The callbacks for this PWM + */ +struct pwm_chip { + int pwm_id; + const char *label; + struct pwm_ops *ops; +}; + +int pwmchip_add(struct pwm_chip *chip); +int pwmchip_remove(struct pwm_chip *chip); +#endif + #endif /* __LINUX_PWM_H */