ARM: dts: aspeed: gpio controller register range
Subsystems (2)
PATCH 1/1 ARM: dts: aspeed: gpio controller register range
Instead of 4k, it should be 512 because the latter part of the memory region are registers for the serial gpio driver.
--- arch/arm/boot/dts/aspeed-g4.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/aspeed-g4.dtsi +1 -1
@@ -231,7 +231,7 @@ #gpio-cells = <2>; gpio-controller; compatible = "aspeed,ast2400-gpio"; - reg = <0x1e780000 0x1000>; + reg = <0x1e780000 0x200>; interrupts = <20>; interrupt-controller; gpio-ranges = <&pinctrl 0 0 220>;
Signature · 1 line
2.15.0.448.gf294e3d99a-goog
Can I get a status on this patch? The device datasheet has 4K reserved for the range, but only the first 512 are registers controlled by this driver. The rest will be handled by a separate driver. Or at least that's how it's planned.
Quoted 25 lines · Patrick Venture
Instead of 4k, it should be 512 because the latter part of the memory region are registers for the serial gpio driver.
Signed-off-by Patrick Venture <[email protected]>--- arch/arm/boot/dts/aspeed-g4.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/dts/aspeed-g4.dtsi +1
-2@@ -231,7 +231,7 @@ #gpio-cells = <2>; gpio-controller; compatible = "aspeed,ast2400-gpio"; - reg = <0x1e780000 0x1000>; + reg = <0x1e780000 0x200>; interrupts = <20>; interrupt-controller; gpio-ranges = <&pinctrl 0 0 220>; --2.15.0.448.gf294e3d99a-goog
Joel Stanley
maintainerQuoted 4 lines · Patrick Venture
Can I get a status on this patch? The device datasheet has 4K reserved for the range, but only the first 512 are registers controlled by this driver. The rest will be handled by a separate driver. Or at least that's how it's planned.
+cc the people who have worked on this.
I think that's okay, but lets see what the others have to say.
Do you have a driver for the rest of the GPIOs you can submit? I'd like to see that code so we can decide if there is good reason to make that completely separate, and not part of the existing driver.
This work is best done upstream, as you will need to make the same arguments there.
Cheers,
Joel
Quoted 27 lines · Patrick Venture
Quoted 25 lines
Instead of 4k, it should be 512 because the latter part of the memory region are registers for the serial gpio driver.
Signed-off-by Patrick Venture <[email protected]>--- arch/arm/boot/dts/aspeed-g4.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/dts/aspeed-g4.dtsi +1
-2@@ -231,7 +231,7 @@ #gpio-cells = <2>; gpio-controller; compatible = "aspeed,ast2400-gpio"; - reg = <0x1e780000 0x1000>; + reg = <0x1e780000 0x200>; interrupts = <20>; interrupt-controller; gpio-ranges = <&pinctrl 0 0 220>; --2.15.0.448.gf294e3d99a-goog
Quoted 8 lines · Joel Stanley
Quoted 4 lines · Patrick Venture
Can I get a status on this patch? The device datasheet has 4K reserved for the range, but only the first 512 are registers controlled by this driver. The rest will be handled by a separate driver. Or at least that's how it's planned.
+cc the people who have worked on this.
Quoted 8 lines
I think that's okay, but lets see what the others have to say.
Do you have a driver for the rest of the GPIOs you can submit? I'd like to see that code so we can decide if there is good reason to make that completely separate, and not part of the existing driver.
This work is best done upstream, as you will need to make the same arguments there.
I should have a driver ready sometime late next week that I'll be sending upstream first. I still need to finish up a few loose ends on it. It wouldn't be unreasonable to have them in the same driver, you'd just end up with two groups of gpios that are handled differently, and extra soft-irqs. That said, if you have two large groups of things that need to be treated differently, then that's somewhat of an argument for having them separate. :)
I'd actually appreciate hearing some thoughts at this point on which way to go, since merging the serial handling into the main driver will require re-working a few things -- I'd rather take the extra time to handle that merge if it's going to head in that direction anyway.
I have read through the gpio-aspeed driver before, as it was a bit useful as a guide, and I could see it getting a bit cluttered with also handling the serial gpios.
Thoughts?
Quoted 32 lines
Cheers,
Joel
Quoted 27 lines · Patrick Venture
Quoted 25 lines
Instead of 4k, it should be 512 because the latter part of the memory region are registers for the serial gpio driver.
Signed-off-by Patrick Venture <[email protected]>--- arch/arm/boot/dts/aspeed-g4.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/dts/aspeed-g4.dtsi +1
-2@@ -231,7 +231,7 @@ #gpio-cells = <2>; gpio-controller; compatible = "aspeed,ast2400-gpio"; - reg = <0x1e780000 0x1000>; + reg = <0x1e780000 0x200>; interrupts = <20>; interrupt-controller; gpio-ranges = <&pinctrl 0 0 220>; --2.15.0.448.gf294e3d99a-goog
Joel Stanley
maintainerQuoted 26 lines · Patrick Venture
Quoted 8 lines · Joel Stanley
Quoted 4 lines · Patrick Venture
Can I get a status on this patch? The device datasheet has 4K reserved for the range, but only the first 512 are registers controlled by this driver. The rest will be handled by a separate driver. Or at least that's how it's planned.
+cc the people who have worked on this.
Quoted 8 lines
I think that's okay, but lets see what the others have to say.
Do you have a driver for the rest of the GPIOs you can submit? I'd like to see that code so we can decide if there is good reason to make that completely separate, and not part of the existing driver.
This work is best done upstream, as you will need to make the same arguments there.
I should have a driver ready sometime late next week that I'll be sending upstream first. I still need to finish up a few loose ends on it. It wouldn't be unreasonable to have them in the same driver, you'd just end up with two groups of gpios that are handled differently, and extra soft-irqs. That said, if you have two large groups of things that need to be treated differently, then that's somewhat of an argument for having them separate. :)
Quoted 8 lines
I'd actually appreciate hearing some thoughts at this point on which way to go, since merging the serial handling into the main driver will require re-working a few things -- I'd rather take the extra time to handle that merge if it's going to head in that direction anyway.
I have read through the gpio-aspeed driver before, as it was a bit useful as a guide, and I could see it getting a bit cluttered with also handling the serial gpios.
It sounds like a call that could go either way. If you think it would be easier to maintain two separate drivers then we can go that direction.
I'd suggest discussing it upstream.
The best way to discuss it would be in the form of a patch. Even if it's not complete, post it as a RFC.
Cheers,
Joel
Thanks for the feedback. I'm still working on getting a few kinks worked out in the driver, for instance, verifying that waiting on the rising edge for the J1 gpio (on the ast2400) is required for stable writing -- which has been true. We were actually seeing the serial gpios wedge to a specific value on the quanta-q71l board, but having it wait on that prevents this case... so more work is going into verifying behaviors and caveats. I'm without a motherboard that has serial gpios and the ast2500 for testing...
Thanks,
Patrick
Quoted 48 lines · Joel Stanley
Quoted 26 lines · Patrick Venture
Quoted 8 lines · Joel Stanley
Quoted 4 lines · Patrick Venture
Can I get a status on this patch? The device datasheet has 4K reserved for the range, but only the first 512 are registers controlled by this driver. The rest will be handled by a separate driver. Or at least that's how it's planned.
+cc the people who have worked on this.
Quoted 8 lines
I think that's okay, but lets see what the others have to say.
Do you have a driver for the rest of the GPIOs you can submit? I'd like to see that code so we can decide if there is good reason to make that completely separate, and not part of the existing driver.
This work is best done upstream, as you will need to make the same arguments there.
I should have a driver ready sometime late next week that I'll be sending upstream first. I still need to finish up a few loose ends on it. It wouldn't be unreasonable to have them in the same driver, you'd just end up with two groups of gpios that are handled differently, and extra soft-irqs. That said, if you have two large groups of things that need to be treated differently, then that's somewhat of an argument for having them separate. :)
Quoted 8 lines
I'd actually appreciate hearing some thoughts at this point on which way to go, since merging the serial handling into the main driver will require re-working a few things -- I'd rather take the extra time to handle that merge if it's going to head in that direction anyway.
I have read through the gpio-aspeed driver before, as it was a bit useful as a guide, and I could see it getting a bit cluttered with also handling the serial gpios.
It sounds like a call that could go either way. If you think it would be easier to maintain two separate drivers then we can go that direction.
I'd suggest discussing it upstream.
The best way to discuss it would be in the form of a patch. Even if it's not complete, post it as a RFC.
Cheers,
Joel
Joel Stanley
maintainerQuoted 25 lines · Patrick Venture
Thanks,
PatrickQuoted 11 lines · Joel Stanley
Quoted 9 lines · Patrick Venture
On Wed, Nov 29, 2017 at 7:53 PM, Joel Stanley <[email protected]> wrote:
I'd actually appreciate hearing some thoughts at this point on which way to go, since merging the serial handling into the main driver will require re-working a few things -- I'd rather take the extra time to handle that merge if it's going to head in that direction anyway.I have read through the gpio-aspeed driver before, as it was a bit useful as a guide, and I could see it getting a bit cluttered with also handling the serial gpios.
Quoted 8 lines
It sounds like a call that could go either way. If you think it would be easier to maintain two separate drivers then we can go that direction.
I'd suggest discussing it upstream.
The best way to discuss it would be in the form of a patch. Even if it's not complete, post it as a RFC.
Quoted 8 lines
Thanks for the feedback. I'm still working on getting a few kinks worked out in the driver, for instance, verifying that waiting on the rising edge for the J1 gpio (on the ast2400) is required for stable writing -- which has been true. We were actually seeing the serial gpios wedge to a specific value on the quanta-q71l board, but having it wait on that prevents this case... so more work is going into verifying behaviors and caveats. I'm without a motherboard that has serial gpios and the ast2500 for testing...
This is a good example of where sending a RFC is the right thing to do
- it doesn't matter that the code isn't functioning, but it will allow review of the structure.
I encourage you to send it out as you have it now.
Cheers,
Joel