gpio: dwapb: Synopsys Designware GPIO

gpio: dwapb: Synopsys Designware GPIO

[Dinh Nguyen]

Hi Jamie,

The socfpga platform is making use of the Synopsys Designware APB GPIO
module.
We found your patches to support them, but I could not figure out why
they weren't
merged upstream.

http://thread.gmane.org/gmane.linux.kernel/1234331

We made a few changes for the IRQs to work, and was wondering if you
want to continue
to take it upstream or we can?

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Steffen Trumtrar]

Hi Dinh,

On Wed, Oct 30, 2013 at 05:46:00PM -0500, Dinh Nguyen wrote:
> Hi Jamie,
> 
> The socfpga platform is making use of the Synopsys Designware APB GPIO
> module.
> We found your patches to support them, but I could not figure out why
> they weren't
> merged upstream.
> 
> http://thread.gmane.org/gmane.linux.kernel/1234331
> 
> We made a few changes for the IRQs to work, and was wondering if you
> want to continue
> to take it upstream or we can?
> 

I figured those patches were lost behind and started just yesterday to
revitalize Jamies work. Although, I think the bank stuff doesn't seem
necessary as the core knows all that stuff by itself AFAIK.
But, I don't have access to the Synopsys datasheet and must go by what
I can make out of the hps.html (which is really inconvenient to use if you
ask me).

Do you have that updated driver somewhere to look at? Maybe I can then just
do something else. The gpio-dw.c from rocketboards/socfpga-3.11 doesn't look
like it is based on Jamies work.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[delicious quinoa]

On Thu, Oct 31, 2013 at 10:18 AM, Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Yes, we'll post a patch up on rocketboards-next and maybe we can post them
> to the mailing list too?
>
Hi Steffen,

I've posted a branch on rocketboards.orgs' linux-socfpga-next.git.
The branch name is dwapb-gpio-3.11.

The top five patches are:
1. Remove the Altera gpio-dw.c driver
2 & 3.  Cherrypick Jamie's stuff from his git repo
4. Enable gpio-dwapb in our defconfig and dts
5. This is the main patch here: use irq_domain_add_linear for
gpio-dwapb.c plus a few bug fixes.

If you'd rather see that last patch on the mailing list, I can post it
there for review.

Alan Tull
Altera Corporation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Steffen Trumtrar]

Hi Alan,

On Thu, Oct 31, 2013 at 10:34:50AM -0500, delicious quinoa wrote:
> On Thu, Oct 31, 2013 at 10:18 AM, Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Yes, we'll post a patch up on rocketboards-next and maybe we can post them
> > to the mailing list too?
> >
> Hi Steffen,
> 
> I've posted a branch on rocketboards.orgs' linux-socfpga-next.git.
> The branch name is dwapb-gpio-3.11.
> 

good. I will have a look.

> The top five patches are:
> 1. Remove the Altera gpio-dw.c driver
> 2 & 3.  Cherrypick Jamie's stuff from his git repo
> 4. Enable gpio-dwapb in our defconfig and dts
> 5. This is the main patch here: use irq_domain_add_linear for
> gpio-dwapb.c plus a few bug fixes.
> 
> If you'd rather see that last patch on the mailing list, I can post it
> there for review.
> 

I wonder if we want to really keep the binding as it is proposed by
Jamie. Do we really win anything by having to specify the banks in the
DT? In my version I get the number of ports, width etc. from the config registers
of the device. I think everything that the device knows itself and can be read
out at runtime shouldn't be specified in the DT.
And it seems that the binding was never merged, so I guess we can change it.

What do you think? I'd then finish up my driver and rebase your irq patch onto
it.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Jamie Iles]

Hi Steffen,

On Thu, Oct 31, 2013 at 04:58:03PM +0100, Steffen Trumtrar wrote:
> Hi Alan,
> 
> On Thu, Oct 31, 2013 at 10:34:50AM -0500, delicious quinoa wrote:
> > On Thu, Oct 31, 2013 at 10:18 AM, Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > Yes, we'll post a patch up on rocketboards-next and maybe we can post them
> > > to the mailing list too?
> > >
> > Hi Steffen,
> > 
> > I've posted a branch on rocketboards.orgs' linux-socfpga-next.git.
> > The branch name is dwapb-gpio-3.11.
> > 
> 
> good. I will have a look.
> 
> > The top five patches are:
> > 1. Remove the Altera gpio-dw.c driver
> > 2 & 3.  Cherrypick Jamie's stuff from his git repo
> > 4. Enable gpio-dwapb in our defconfig and dts
> > 5. This is the main patch here: use irq_domain_add_linear for
> > gpio-dwapb.c plus a few bug fixes.
> > 
> > If you'd rather see that last patch on the mailing list, I can post it
> > there for review.
> > 
> 
> I wonder if we want to really keep the binding as it is proposed by
> Jamie. Do we really win anything by having to specify the banks in the
> DT? In my version I get the number of ports, width etc. from the config registers
> of the device. I think everything that the device knows itself and can be read
> out at runtime shouldn't be specified in the DT.
> And it seems that the binding was never merged, so I guess we can change it.

The reason for having it split into banks is that for hardware that has 
a mixture of bank sizes (odd hardware admittedly, but that includes 
hardware that I was writing the driver for), we had a setup like 16 pins 
on bank A, 16 on B, 1 on C and 16 on D where each bank could have a 
maximum of 32, so converting from a data sheet to GPIO number is not 
obvious.  Grant Likely suggested representing the banks as different 
devices, so that's how I created the binding.

Jamie
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Steffen Trumtrar]

Hi Jamie,

On Thu, Oct 31, 2013 at 04:07:23PM +0000, Jamie Iles wrote:
> Hi Steffen,
> 
> On Thu, Oct 31, 2013 at 04:58:03PM +0100, Steffen Trumtrar wrote:
> > Hi Alan,
> > 
> > On Thu, Oct 31, 2013 at 10:34:50AM -0500, delicious quinoa wrote:
> > > On Thu, Oct 31, 2013 at 10:18 AM, Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > Yes, we'll post a patch up on rocketboards-next and maybe we can post them
> > > > to the mailing list too?
> > > >
> > > Hi Steffen,
> > > 
> > > I've posted a branch on rocketboards.orgs' linux-socfpga-next.git.
> > > The branch name is dwapb-gpio-3.11.
> > > 
> > 
> > good. I will have a look.
> > 
> > > The top five patches are:
> > > 1. Remove the Altera gpio-dw.c driver
> > > 2 & 3.  Cherrypick Jamie's stuff from his git repo
> > > 4. Enable gpio-dwapb in our defconfig and dts
> > > 5. This is the main patch here: use irq_domain_add_linear for
> > > gpio-dwapb.c plus a few bug fixes.
> > > 
> > > If you'd rather see that last patch on the mailing list, I can post it
> > > there for review.
> > > 
> > 
> > I wonder if we want to really keep the binding as it is proposed by
> > Jamie. Do we really win anything by having to specify the banks in the
> > DT? In my version I get the number of ports, width etc. from the config registers
> > of the device. I think everything that the device knows itself and can be read
> > out at runtime shouldn't be specified in the DT.
> > And it seems that the binding was never merged, so I guess we can change it.
> 
> The reason for having it split into banks is that for hardware that has 
> a mixture of bank sizes (odd hardware admittedly, but that includes 
> hardware that I was writing the driver for), we had a setup like 16 pins 
> on bank A, 16 on B, 1 on C and 16 on D where each bank could have a 
> maximum of 32, so converting from a data sheet to GPIO number is not 
> obvious.  Grant Likely suggested representing the banks as different 
> devices, so that's how I created the binding.
> 

I have no problem with the binding, if it is worth it. Do you mean you had
hardware where the gpio lines where not connected? Or maybe you had an older
version of the IP core?
In the Socfpga case there is gpio_config_reg2, which specifies the width
for every of the 4 ports. So I thought I use those values to describe my
hardware. The Socfpga however only uses the first bank of its three GPIO cores,
so I wouldn't be able to test if the code works for more than just that.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Jamie Iles]

On Thu, Oct 31, 2013 at 05:17:39PM +0100, Steffen Trumtrar wrote:
> On Thu, Oct 31, 2013 at 04:07:23PM +0000, Jamie Iles wrote:
> > The reason for having it split into banks is that for hardware that 
> > has a mixture of bank sizes (odd hardware admittedly, but that 
> > includes hardware that I was writing the driver for), we had a setup 
> > like 16 pins on bank A, 16 on B, 1 on C and 16 on D where each bank 
> > could have a maximum of 32, so converting from a data sheet to GPIO 
> > number is not obvious.  Grant Likely suggested representing the 
> > banks as different devices, so that's how I created the binding.
> > 
> 
> I have no problem with the binding, if it is worth it. Do you mean you had
> hardware where the gpio lines where not connected? Or maybe you had an older
> version of the IP core?

Not even unconnected, the IP was configured to have banks of varying 
width!  Perhaps it was something to do with the physical pads that they 
were connected to having different I/O standards or something.

> In the Socfpga case there is gpio_config_reg2, which specifies the width
> for every of the 4 ports. So I thought I use those values to describe my
> hardware. The Socfpga however only uses the first bank of its three GPIO cores,
> so I wouldn't be able to test if the code works for more than just that.

I think that if there was a guarantee that all banks apart from the last 
were configured to equal width then representing the controller as a 
single device would be right, but given that it isn't always the case 
then seperate devices for each bank is probably the most sensible.

That said I'd personally be happy either way to see the patches merged!

Jamie
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Sebastian Hesselbarth]

On 10/31/2013 05:17 PM, Steffen Trumtrar wrote:
> On Thu, Oct 31, 2013 at 04:07:23PM +0000, Jamie Iles wrote:
>> On Thu, Oct 31, 2013 at 04:58:03PM +0100, Steffen Trumtrar wrote:
>>> I wonder if we want to really keep the binding as it is proposed by
>>> Jamie. Do we really win anything by having to specify the banks in the
>>> DT? In my version I get the number of ports, width etc. from the config registers
>>> of the device. I think everything that the device knows itself and can be read
>>> out at runtime shouldn't be specified in the DT.
>>> And it seems that the binding was never merged, so I guess we can change it.
>>
>> The reason for having it split into banks is that for hardware that has
>> a mixture of bank sizes (odd hardware admittedly, but that includes
>> hardware that I was writing the driver for), we had a setup like 16 pins
>> on bank A, 16 on B, 1 on C and 16 on D where each bank could have a
>> maximum of 32, so converting from a data sheet to GPIO number is not
>> obvious.  Grant Likely suggested representing the banks as different
>> devices, so that's how I created the binding.
>
> I have no problem with the binding, if it is worth it. Do you mean you had
> hardware where the gpio lines where not connected? Or maybe you had an older
> version of the IP core?
> In the Socfpga case there is gpio_config_reg2, which specifies the width
> for every of the 4 ports. So I thought I use those values to describe my
> hardware. The Socfpga however only uses the first bank of its three GPIO cores,
> so I wouldn't be able to test if the code works for more than just that.

Having talked with Heiko at ELCE, both upcoming mach-berlin and
mach-rockchip will be using dw-apb-gpio as GPIO driver. Unfortunately,
while berlin has the CONFIG[12] registers, rockchip has not. So there
must be bindings to at least overwrite the obvious settings derived
from that registers.

Also, IIRC Heiko prefers to use platform_data driven registration as it
is tightly coupled with rk's pinmux. So there should really be support
for that and DT parsing on top that fills platform_data.

Can you please resend the whole set on list so we can review it again
with more machs involved?

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gpio: dwapb: Synopsys Designware GPIO

[Steffen Trumtrar]

On Thu, Oct 31, 2013 at 06:21:46PM +0100, Sebastian Hesselbarth wrote:
> On 10/31/2013 05:17 PM, Steffen Trumtrar wrote:
> >On Thu, Oct 31, 2013 at 04:07:23PM +0000, Jamie Iles wrote:
> >>On Thu, Oct 31, 2013 at 04:58:03PM +0100, Steffen Trumtrar wrote:
> >>>I wonder if we want to really keep the binding as it is proposed by
> >>>Jamie. Do we really win anything by having to specify the banks in the
> >>>DT? In my version I get the number of ports, width etc. from the config registers
> >>>of the device. I think everything that the device knows itself and can be read
> >>>out at runtime shouldn't be specified in the DT.
> >>>And it seems that the binding was never merged, so I guess we can change it.
> >>
> >>The reason for having it split into banks is that for hardware that has
> >>a mixture of bank sizes (odd hardware admittedly, but that includes
> >>hardware that I was writing the driver for), we had a setup like 16 pins
> >>on bank A, 16 on B, 1 on C and 16 on D where each bank could have a
> >>maximum of 32, so converting from a data sheet to GPIO number is not
> >>obvious.  Grant Likely suggested representing the banks as different
> >>devices, so that's how I created the binding.
> >
> >I have no problem with the binding, if it is worth it. Do you mean you had
> >hardware where the gpio lines where not connected? Or maybe you had an older
> >version of the IP core?
> >In the Socfpga case there is gpio_config_reg2, which specifies the width
> >for every of the 4 ports. So I thought I use those values to describe my
> >hardware. The Socfpga however only uses the first bank of its three GPIO cores,
> >so I wouldn't be able to test if the code works for more than just that.
> 
> Having talked with Heiko at ELCE, both upcoming mach-berlin and
> mach-rockchip will be using dw-apb-gpio as GPIO driver. Unfortunately,
> while berlin has the CONFIG[12] registers, rockchip has not. So there
> must be bindings to at least overwrite the obvious settings derived
> from that registers.
> 

Well, okay :-( It looks like the original binding is a must than.

> Also, IIRC Heiko prefers to use platform_data driven registration as it
> is tightly coupled with rk's pinmux. So there should really be support
> for that and DT parsing on top that fills platform_data.
> 

If it's up to me, that may be added later when needed. But I guess I'm
out and will leave that to Alan/Dinh as they already have a more
appropriate series.

> Can you please resend the whole set on list so we can review it again
> with more machs involved?
> 

Dito.

Thanks,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html