On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
[...]
> +#include <linux/platform_data/ti-pwmss.h>
[...]
> +static struct pwmss_platform_data am33xx_data = {
> + .has_configspace = true,
> +};
This structure is defined in a public header. I don't see why it has to,
given that it's only used to associate some data with an of_device_id
entry below. Since AM33xx never had anything but OF support in the
mainline kernel I don't think we should add platform data.
> +#ifdef CONFIG_OF
> +static const struct of_device_id ecap_of_match[] = {
> + {
> + .compatible = "ti,am33xx-ecap",
> + .data = &am33xx_data,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ecap_of_match);
> +#endif
> +
I don't quite see why we need to pass this platform data to the device
at all since there is no other variant anyway. I think this should only
be added if this turns out to be required at some point.
> static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> {
> int ret;
> struct resource *r;
> struct clk *clk;
> struct ecap_pwm_chip *pc;
> + struct pwmss_platform_data *pdata = pdev->dev.platform_data;
> + const struct of_device_id *match;
> + u16 regval;
> + struct pinctrl *pinctrl;
> +
> + match = of_match_device(of_match_ptr(ecap_of_match), &pdev->dev);
I've never seen of_match_ptr() used this way. Maybe you should think
about not #ifdef'ing OF specific code at all. That way ecap_of_match
will always exist and you could use the IS_ENABLED() macro to
conditionalize at compile time. The compiler will probably be clever
enough to optimize the ecap_of_match away if OF is not enabled.
Given my comment earlier, since AM33xx is OF only you might just as well
add a depends on OF to this driver and things will become a lot easier.
> +
> + if (match)
> + pdata = (struct pwmss_platform_data *)match->data;
> +
> + pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> + if (IS_ERR(pinctrl))
> + dev_warn(&pdev->dev, "failed to configure pins from driver\n");
>
> pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
> if (!pc) {
> @@ -211,6 +268,8 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
>
> pc->chip.dev = &pdev->dev;
> pc->chip.ops = &ecap_pwm_ops;
> + pc->chip.of_xlate = of_ecap_xlate;
> + pc->chip.of_pwm_n_cells = PWM_CELL_SIZE;
> pc->chip.base = -1;
> pc->chip.npwm = 1;
>
> @@ -231,13 +290,56 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> }
>
> pm_runtime_enable(&pdev->dev);
> +
> + /*
> + * Some platform has extra PWM-subsystem common config space
> + * and requires special handling of clock gating.
> + */
> + if (pdata && pdata->has_configspace) {
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!r) {
> + dev_err(&pdev->dev, "no memory resource defined\n");
> + ret = -ENODEV;
> + goto err_disable_clock;
> + }
> +
> + pc->config_base = devm_ioremap(&pdev->dev, r->start,
> + resource_size(r));
> + if (!pc->config_base) {
> + dev_err(&pdev->dev, "failed to ioremap() registers\n");
> + ret = -EADDRNOTAVAIL;
> + goto err_disable_clock;
> + }
Isn't this missing a request_mem_region()? I assume you don't do that
here because you need the same region in the EHRPWM driver, right? This
should be indication enough that the design is not right here. I think
we need a proper abstraction here. Can this not be done via PM runtime
support? If not, maybe this should be represented by clock objects since
the bit obviously enables a clock.
> +
> + /* Enable ECAP clock gating at PWM-subsystem common config */
> + pm_runtime_get_sync(&pdev->dev);
> + regval = readw(pc->config_base + PWMSS_CLKCONFIG);
> + regval |= PWMSS_ECAP_CLK_EN;
> + writew(regval, pc->config_base + PWMSS_CLKCONFIG);
> + pm_runtime_put_sync(&pdev->dev);
> + }
> +
> platform_set_drvdata(pdev, pc);
> return 0;
> +
> +err_disable_clock:
> + pm_runtime_disable(&pdev->dev);
> + return ret;
> }
>
> static int __devexit ecap_pwm_remove(struct platform_device *pdev)
> {
> struct ecap_pwm_chip *pc = platform_get_drvdata(pdev);
> + u16 regval;
> +
> + if (pc->config_base) {
> + /* Disable ECAP clock gating at PWM-subsystem common config */
> + pm_runtime_get_sync(&pdev->dev);
> + regval = readw(pc->config_base + PWMSS_CLKCONFIG);
> + regval &= ~PWMSS_ECAP_CLK_EN;
> + writew(regval, pc->config_base + PWMSS_CLKCONFIG);
> + pm_runtime_put_sync(&pdev->dev);
> + }
>
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> @@ -247,6 +349,9 @@ static int __devexit ecap_pwm_remove(struct platform_device *pdev)
> static struct platform_driver ecap_pwm_driver = {
> .driver = {
> .name = "ecap",
> +#ifdef CONFIG_OF
> + .of_match_table = of_match_ptr(ecap_of_match),
> +#endif
If you use of_match_ptr() you don't need the additional #ifdef
CONFIG_OF. But as I already said I don't think you need this at all
anyway. You should really just depend on OF and be done with it.
Thierry
Hi Thierry,
On 10/2/2012 11:30 AM, Thierry Reding wrote:
> On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
> [...]
>> +#include <linux/platform_data/ti-pwmss.h>
> [...]
>> +static struct pwmss_platform_data am33xx_data = {
>> + .has_configspace = true,
>> +};
>
> This structure is defined in a public header. I don't see why it has to,
> given that it's only used to associate some data with an of_device_id
> entry below. Since AM33xx never had anything but OF support in the
> mainline kernel I don't think we should add platform data.
Avinash probably introduced platform data because the same PWM IP is
used in older DaVinci family SoCs (DA830 and DA850) which are not
converted to DT. There are existing boards for those SoCs (supported in
mainline) which could benefit from this driver.
Thanks,
Sekhar
On Tue, Oct 02, 2012 at 12:46:16PM +0530, Sekhar Nori wrote:
> Hi Thierry,
>
> On 10/2/2012 11:30 AM, Thierry Reding wrote:
> > On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
> > [...]
> >> +#include <linux/platform_data/ti-pwmss.h>
> > [...]
> >> +static struct pwmss_platform_data am33xx_data = {
> >> + .has_configspace = true,
> >> +};
> >
> > This structure is defined in a public header. I don't see why it has to,
> > given that it's only used to associate some data with an of_device_id
> > entry below. Since AM33xx never had anything but OF support in the
> > mainline kernel I don't think we should add platform data.
>
> Avinash probably introduced platform data because the same PWM IP is
> used in older DaVinci family SoCs (DA830 and DA850) which are not
> converted to DT. There are existing boards for those SoCs (supported in
> mainline) which could benefit from this driver.
Okay. If that's the case platform data should be added along with
support for those SoCs. Ideally, of course, the DaVinci boards would be
converted to DT first so we wouldn't have to introduce platform data
just to get rid of it when the conversion does take place. Until now it
seems like the boards have managed to get by without PWM support so
maybe they just don't use or need it?
Thierry
On 10/2/2012 1:37 PM, Thierry Reding wrote:
> On Tue, Oct 02, 2012 at 12:46:16PM +0530, Sekhar Nori wrote:
>> Hi Thierry,
>>
>> On 10/2/2012 11:30 AM, Thierry Reding wrote:
>>> On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
>>> [...]
>>>> +#include <linux/platform_data/ti-pwmss.h>
>>> [...]
>>>> +static struct pwmss_platform_data am33xx_data = {
>>>> + .has_configspace = true,
>>>> +};
>>>
>>> This structure is defined in a public header. I don't see why it has to,
>>> given that it's only used to associate some data with an of_device_id
>>> entry below. Since AM33xx never had anything but OF support in the
>>> mainline kernel I don't think we should add platform data.
>>
>> Avinash probably introduced platform data because the same PWM IP is
>> used in older DaVinci family SoCs (DA830 and DA850) which are not
>> converted to DT. There are existing boards for those SoCs (supported in
>> mainline) which could benefit from this driver.
>
> Okay. If that's the case platform data should be added along with
> support for those SoCs. Ideally, of course, the DaVinci boards would be
> converted to DT first so we wouldn't have to introduce platform data
> just to get rid of it when the conversion does take place. Until now it
> seems like the boards have managed to get by without PWM support so
> maybe they just don't use or need it?
On top of my head, the DA850 EVM uses PWM for LCD backlight control.
Attempts were made in the past to support PWM on this board back when
Bill Gatliff was attempting to get a framework accepted. So, I guess the
boards were waiting for a framework to materialize ;-)
I posted some patches to add basic DT support for DA850 EVM. It will be
a while before the entire board is converted over. Also, platforms like
DA830 have lesser number of users so it will likely not be converted at all.
It is fair to add platform data along with SoC support. Thanks!
Regards,
Sekhar
On Tue, Oct 02, 2012 at 11:30:14, Thierry Reding wrote:
> On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
> [...]
> > +#include <linux/platform_data/ti-pwmss.h>
> [...]
> > +static struct pwmss_platform_data am33xx_data = {
> > + .has_configspace = true,
> > +};
>
> This structure is defined in a public header. I don't see why it has to,
> given that it's only used to associate some data with an of_device_id
> entry below. Since AM33xx never had anything but OF support in the
> mainline kernel I don't think we should add platform data.
I will correct it. I was going through Sekhar's reply.
>
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id ecap_of_match[] = {
> > + {
> > + .compatible = "ti,am33xx-ecap",
> > + .data = &am33xx_data,
> > + },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ecap_of_match);
> > +#endif
> > +
>
> I don't quite see why we need to pass this platform data to the device
> at all since there is no other variant anyway. I think this should only
> be added if this turns out to be required at some point.
>
> > static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> > {
> > int ret;
> > struct resource *r;
> > struct clk *clk;
> > struct ecap_pwm_chip *pc;
> > + struct pwmss_platform_data *pdata = pdev->dev.platform_data;
> > + const struct of_device_id *match;
> > + u16 regval;
> > + struct pinctrl *pinctrl;
> > +
> > + match = of_match_device(of_match_ptr(ecap_of_match), &pdev->dev);
>
> I've never seen of_match_ptr() used this way. Maybe you should think
> about not #ifdef'ing OF specific code at all. That way ecap_of_match
> will always exist and you could use the IS_ENABLED() macro to
> conditionalize at compile time. The compiler will probably be clever
> enough to optimize the ecap_of_match away if OF is not enabled.
I will correct it.
>
> Given my comment earlier, since AM33xx is OF only you might just as well
> add a depends on OF to this driver and things will become a lot easier.
I will check & correct it.
>
> > +
> > + if (match)
> > + pdata = (struct pwmss_platform_data *)match->data;
> > +
> > + pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> > + if (IS_ERR(pinctrl))
> > + dev_warn(&pdev->dev, "failed to configure pins from driver\n");
> >
> > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
> > if (!pc) {
> > @@ -211,6 +268,8 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> >
> > pc->chip.dev = &pdev->dev;
> > pc->chip.ops = &ecap_pwm_ops;
> > + pc->chip.of_xlate = of_ecap_xlate;
> > + pc->chip.of_pwm_n_cells = PWM_CELL_SIZE;
> > pc->chip.base = -1;
> > pc->chip.npwm = 1;
> >
> > @@ -231,13 +290,56 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> > }
> >
> > pm_runtime_enable(&pdev->dev);
> > +
> > + /*
> > + * Some platform has extra PWM-subsystem common config space
> > + * and requires special handling of clock gating.
> > + */
> > + if (pdata && pdata->has_configspace) {
> > + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + if (!r) {
> > + dev_err(&pdev->dev, "no memory resource defined\n");
> > + ret = -ENODEV;
> > + goto err_disable_clock;
> > + }
> > +
> > + pc->config_base = devm_ioremap(&pdev->dev, r->start,
> > + resource_size(r));
> > + if (!pc->config_base) {
> > + dev_err(&pdev->dev, "failed to ioremap() registers\n");
> > + ret = -EADDRNOTAVAIL;
> > + goto err_disable_clock;
> > + }
>
> Isn't this missing a request_mem_region()? I assume you don't do that
> here because you need the same region in the EHRPWM driver, right?
request_mem_region() is avoided as this region is shared across PWM
sub modules ECAP & EHRPWM.
> This should be indication enough that the design is not right here.
> I think we need a proper abstraction here. Can this not be done via
> PM runtime support? If not, maybe this should be represented by
> clock objects since the bit obviously enables a clock.
It is not done as part of PM runtime as this is has nothing to
do with clock tree of the SOC. The bits we were enabling here
should consider as an enable of the individual sub module as
part of IP integration. Hence we were handling these subsystem
module enable in the driver itself.
>
> > +
> > + /* Enable ECAP clock gating at PWM-subsystem common config */
> > + pm_runtime_get_sync(&pdev->dev);
> > + regval = readw(pc->config_base + PWMSS_CLKCONFIG);
> > + regval |= PWMSS_ECAP_CLK_EN;
> > + writew(regval, pc->config_base + PWMSS_CLKCONFIG);
> > + pm_runtime_put_sync(&pdev->dev);
> > + }
> > +
> > platform_set_drvdata(pdev, pc);
> > return 0;
> > +
> > +err_disable_clock:
> > + pm_runtime_disable(&pdev->dev);
> > + return ret;
> > }
> >
> > static int __devexit ecap_pwm_remove(struct platform_device *pdev)
> > {
> > struct ecap_pwm_chip *pc = platform_get_drvdata(pdev);
> > + u16 regval;
> > +
> > + if (pc->config_base) {
> > + /* Disable ECAP clock gating at PWM-subsystem common config */
> > + pm_runtime_get_sync(&pdev->dev);
> > + regval = readw(pc->config_base + PWMSS_CLKCONFIG);
> > + regval &= ~PWMSS_ECAP_CLK_EN;
> > + writew(regval, pc->config_base + PWMSS_CLKCONFIG);
> > + pm_runtime_put_sync(&pdev->dev);
> > + }
> >
> > pm_runtime_put_sync(&pdev->dev);
> > pm_runtime_disable(&pdev->dev);
> > @@ -247,6 +349,9 @@ static int __devexit ecap_pwm_remove(struct platform_device *pdev)
> > static struct platform_driver ecap_pwm_driver = {
> > .driver = {
> > .name = "ecap",
> > +#ifdef CONFIG_OF
> > + .of_match_table = of_match_ptr(ecap_of_match),
> > +#endif
>
> If you use of_match_ptr() you don't need the additional #ifdef
> CONFIG_OF. But as I already said I don't think you need this at all
> anyway. You should really just depend on OF and be done with it.
Ok I understood & will correct it.
Thanks
Avinash
>
> Thierry
>
On Mon, Oct 08, 2012 at 01:31:19PM +0000, Philip, Avinash wrote:
> On Tue, Oct 02, 2012 at 11:30:14, Thierry Reding wrote:
> > On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
[...]
> > > @@ -231,13 +290,56 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> > > }
> > >
> > > pm_runtime_enable(&pdev->dev);
> > > +
> > > + /*
> > > + * Some platform has extra PWM-subsystem common config space
> > > + * and requires special handling of clock gating.
> > > + */
> > > + if (pdata && pdata->has_configspace) {
> > > + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > > + if (!r) {
> > > + dev_err(&pdev->dev, "no memory resource defined\n");
> > > + ret = -ENODEV;
> > > + goto err_disable_clock;
> > > + }
> > > +
> > > + pc->config_base = devm_ioremap(&pdev->dev, r->start,
> > > + resource_size(r));
> > > + if (!pc->config_base) {
> > > + dev_err(&pdev->dev, "failed to ioremap() registers\n");
> > > + ret = -EADDRNOTAVAIL;
> > > + goto err_disable_clock;
> > > + }
> >
> > Isn't this missing a request_mem_region()? I assume you don't do that
> > here because you need the same region in the EHRPWM driver, right?
>
> request_mem_region() is avoided as this region is shared across PWM
> sub modules ECAP & EHRPWM.
>
> > This should be indication enough that the design is not right here.
> > I think we need a proper abstraction here. Can this not be done via
> > PM runtime support? If not, maybe this should be represented by
> > clock objects since the bit obviously enables a clock.
>
> It is not done as part of PM runtime as this is has nothing to
> do with clock tree of the SOC. The bits we were enabling here
> should consider as an enable of the individual sub module as
> part of IP integration. Hence we were handling these subsystem
> module enable in the driver itself.
My point remains valid: you shouldn't be able to access the same
register through two different drivers. That's one of the reasons, if
not the only reasen, why the request_mem_region() function exists. I
think you should add some abstraction to provide this functionality to
the drivers. I assume that eventually there will be more than just the
PWM cores that require access to this register.
Thierry
On Mon, Oct 08, 2012 at 19:09:51, Thierry Reding wrote:
> On Mon, Oct 08, 2012 at 01:31:19PM +0000, Philip, Avinash wrote:
> > On Tue, Oct 02, 2012 at 11:30:14, Thierry Reding wrote:
> > > On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
> [...]
> > > > @@ -231,13 +290,56 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> > > > }
> > > >
> > > > pm_runtime_enable(&pdev->dev);
> > > > +
> > > > + /*
> > > > + * Some platform has extra PWM-subsystem common config space
> > > > + * and requires special handling of clock gating.
> > > > + */
> > > > + if (pdata && pdata->has_configspace) {
> > > > + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > > > + if (!r) {
> > > > + dev_err(&pdev->dev, "no memory resource defined\n");
> > > > + ret = -ENODEV;
> > > > + goto err_disable_clock;
> > > > + }
> > > > +
> > > > + pc->config_base = devm_ioremap(&pdev->dev, r->start,
> > > > + resource_size(r));
> > > > + if (!pc->config_base) {
> > > > + dev_err(&pdev->dev, "failed to ioremap() registers\n");
> > > > + ret = -EADDRNOTAVAIL;
> > > > + goto err_disable_clock;
> > > > + }
> > >
> > > Isn't this missing a request_mem_region()? I assume you don't do that
> > > here because you need the same region in the EHRPWM driver, right?
> >
> > request_mem_region() is avoided as this region is shared across PWM
> > sub modules ECAP & EHRPWM.
> >
> > > This should be indication enough that the design is not right here.
> > > I think we need a proper abstraction here. Can this not be done via
> > > PM runtime support? If not, maybe this should be represented by
> > > clock objects since the bit obviously enables a clock.
> >
> > It is not done as part of PM runtime as this is has nothing to
> > do with clock tree of the SOC. The bits we were enabling here
> > should consider as an enable of the individual sub module as
> > part of IP integration. Hence we were handling these subsystem
> > module enable in the driver itself.
>
> My point remains valid: you shouldn't be able to access the same
> register through two different drivers. That's one of the reasons, if
> not the only reasen, why the request_mem_region() function exists. I
> think you should add some abstraction to provide this functionality to
> the drivers. I assume that eventually there will be more than just the
> PWM cores that require access to this register.
Enabling of PWM sub modules from CONFIG space is only present in AM33xx
as part of IP integration (ECAP, EHRPWM & EQEP).
Enabling of sub modules (ECAP, EHRPWM & EQEP) should do in CONFIG space.
Hence sub module drivers are accessing CONFIG space without reserving it
Individually from drivers (request_mem_region()).
Can you describe/point how it can be handled in a separate Abstraction layer
as this is shared across ECAP, EHRPWM & EQEP (EQEP driver is not yet available).
Adding it as part of PM runtime support is not a right place.
In PWM-SS, CONFIG Space is "Shared" across different sub modules and hence
can't be considered as a part of clock tree.
Thanks
Avinash
>
> Thierry
>
On Tue, Oct 09, 2012 at 12:36:53PM +0000, Philip, Avinash wrote:
> On Mon, Oct 08, 2012 at 19:09:51, Thierry Reding wrote:
> > On Mon, Oct 08, 2012 at 01:31:19PM +0000, Philip, Avinash wrote:
> > > On Tue, Oct 02, 2012 at 11:30:14, Thierry Reding wrote:
> > > > On Wed, Sep 26, 2012 at 04:57:42PM +0530, Philip, Avinash wrote:
> > [...]
> > > > > @@ -231,13 +290,56 @@ static int __devinit ecap_pwm_probe(struct platform_device *pdev)
> > > > > }
> > > > >
> > > > > pm_runtime_enable(&pdev->dev);
> > > > > +
> > > > > + /*
> > > > > + * Some platform has extra PWM-subsystem common config space
> > > > > + * and requires special handling of clock gating.
> > > > > + */
> > > > > + if (pdata && pdata->has_configspace) {
> > > > > + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > > > > + if (!r) {
> > > > > + dev_err(&pdev->dev, "no memory resource defined\n");
> > > > > + ret = -ENODEV;
> > > > > + goto err_disable_clock;
> > > > > + }
> > > > > +
> > > > > + pc->config_base = devm_ioremap(&pdev->dev, r->start,
> > > > > + resource_size(r));
> > > > > + if (!pc->config_base) {
> > > > > + dev_err(&pdev->dev, "failed to ioremap() registers\n");
> > > > > + ret = -EADDRNOTAVAIL;
> > > > > + goto err_disable_clock;
> > > > > + }
> > > >
> > > > Isn't this missing a request_mem_region()? I assume you don't do that
> > > > here because you need the same region in the EHRPWM driver, right?
> > >
> > > request_mem_region() is avoided as this region is shared across PWM
> > > sub modules ECAP & EHRPWM.
> > >
> > > > This should be indication enough that the design is not right here.
> > > > I think we need a proper abstraction here. Can this not be done via
> > > > PM runtime support? If not, maybe this should be represented by
> > > > clock objects since the bit obviously enables a clock.
> > >
> > > It is not done as part of PM runtime as this is has nothing to
> > > do with clock tree of the SOC. The bits we were enabling here
> > > should consider as an enable of the individual sub module as
> > > part of IP integration. Hence we were handling these subsystem
> > > module enable in the driver itself.
> >
> > My point remains valid: you shouldn't be able to access the same
> > register through two different drivers. That's one of the reasons, if
> > not the only reasen, why the request_mem_region() function exists. I
> > think you should add some abstraction to provide this functionality to
> > the drivers. I assume that eventually there will be more than just the
> > PWM cores that require access to this register.
>
> Enabling of PWM sub modules from CONFIG space is only present in AM33xx
> as part of IP integration (ECAP, EHRPWM & EQEP).
>
> Enabling of sub modules (ECAP, EHRPWM & EQEP) should do in CONFIG space.
> Hence sub module drivers are accessing CONFIG space without reserving it
> Individually from drivers (request_mem_region()).
>
> Can you describe/point how it can be handled in a separate Abstraction layer
> as this is shared across ECAP, EHRPWM & EQEP (EQEP driver is not yet available).
Perhaps something like a global function to enable and disable the
clocks would be enough. You could for instance have a driver that
requests the config space memory region and provides this global
function so that it can be used by the ECAP, EHRPWM and EQEP.
A more elaborate scheme would possibly involve making the new device a
parent of the ECAP, EHRPWM and EQEP devices and have their drivers
lookup the parent to determine which configurator device to operate on.
Thierry