[PATCH, v1 1/1] gpio: aspeed: Add gpio base address reading
> Hello, > > On Thu, 14 Jan 2021, at 09:08, Hongwei Zhang wrote: > > Add gpio base address reading in the driver; in old code, it just > > returns -1 to gpio->chip.base. > > Why do you want to do this? It feels hacky. The base address only affects the > legacy sysfs number-space, > and even then if you're using the sysfs interface you can discover the base > address for a specific gpiochip > via the associated attribute. For example: > > # cat /sys/bus/platform/devices/1e78.gpio/gpio/gpiochip*/base > 816 > > I feel that you should instead be changing your userspace not to assume a > fixed value. > > Finally, the base value is a linux-specific thing and doesn't belong in the > devicetree, and if it did, you > would also need to update the devicetree binding in Documentation/. > > Cheers, > > Andrew Hi Andrew, Thanks for your review and advice. --Hongwei
[PATCH, v1 0/1] gpio: aspeed: Add gpio base address reading
Dear Reviewer, Add gpio base address reading in the driver; in old code, it just returns -1 to gpio->chip.base. In this patch, the code first try to read base address from of_property_read_u32(), if it fails, then return -1. Hongwei Zhang (1): gpio: aspeed: Add gpio base address reading drivers/gpio/gpio-aspeed.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) -- 2.17.1
[PATCH, v1 1/1] gpio: aspeed: Add gpio base address reading
Add gpio base address reading in the driver; in old code, it just
returns -1 to gpio->chip.base.
Fixes: 7ee2d5b4d4340353 ("ARM: dts: nuvoton: Add Fii Kudo system")
Signed-off-by: Hongwei Zhang
---
drivers/gpio/gpio-aspeed.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-aspeed.c b/drivers/gpio/gpio-aspeed.c
index d07bf2c3f136..4ebe4c40154c 100644
--- a/drivers/gpio/gpio-aspeed.c
+++ b/drivers/gpio/gpio-aspeed.c
@@ -1140,7 +1140,7 @@ static int __init aspeed_gpio_probe(struct
platform_device *pdev)
const struct of_device_id *gpio_id;
struct aspeed_gpio *gpio;
int rc, i, banks, err;
- u32 ngpio;
+ u32 ngpio, base;
gpio = devm_kzalloc(>dev, sizeof(*gpio), GFP_KERNEL);
if (!gpio)
@@ -1179,7 +1179,10 @@ static int __init aspeed_gpio_probe(struct
platform_device *pdev)
gpio->chip.set = aspeed_gpio_set;
gpio->chip.set_config = aspeed_gpio_set_config;
gpio->chip.label = dev_name(>dev);
- gpio->chip.base = -1;
+ err = of_property_read_u32(pdev->dev.of_node, "base", );
+ gpio->chip.base = (u16) base;
+ if (err)
+ gpio->chip.base = -1;
/* Allocate a cache of the output registers */
banks = DIV_ROUND_UP(gpio->chip.ngpio, 32);
--
2.17.1
[Aspeed,ncsi-rx, v2 1/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
>
> From: Jakub Kicinski
> Sent: Monday, December 21, 2020 5:10 PM
> To: Hongwei Zhang
>
> On Mon, 21 Dec 2020 14:40:26 -0500 Hongwei Zhang wrote:
> > When FTGMAC100 driver is used on other NCSI Ethernet controllers, few
>
> When you say NCSI Ethernet controller here you mean the main system NIC,
> right? The MAC on the NCSI
> side is FTGMAC100, correct?
>
Hi Jakub,
The use case for us is the MAC is configured as NCSI, so it provides network
access for both BMC and Host, the Ethernet controller driver is from BMC kernel
side.
please see my response to Dylan in another thread,
<20201215192323.24359-1-hongw...@ami.com>,
he points out the root cause of the issue.
> In that case I'm not sure how user is supposed to control this setting at
> build time. The system NIC is
> often pluggable on the PCIe bus, and can be changed at will.
>
> > controllers have compatible issue, removing FTGMAC100_RXDES0_RX_ERR
> > bit from RXDES0_ANY_ERROR can fix the issue.
> >
> > Fixes: 7ee2d5b4d4340353 ("ARM: dts: nuvoton: Add Fii Kudo system")
>
> Please fix the commit hash, this hash does not exist upstream:
>
will do.
Thanks
--Hongwei
> Commit: 8711d4ef64fa ("net: ftgmac100: Fix AST2600 EVB NCSI RX issue")
> Fixes tag: Fixes: 7ee2d5b4d4340353 ("ARM: dts: nuvoton: Add Fii Kudo
> system")
> Has these problem(s):
> - Target SHA1 does not exist
>
[Aspeed,ncsi-rx, v1 0/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
> > Hi Hongwei, > > The NCSI should run on 3.3V RMII. According your log, you enabled NCSI on > ftgmac100@1e66 which can only support 1.8V I/O voltage. > Did you observe the same error on ftgmac100@1e67 (MAC3) or > ftgmac100@1e69 (MAC4)? > Hi Dylan, Thanks for your review and input, you're correct, this issue is not observed on AST2600 MAC4 (ftgmac100@1e69). Though this issue is caused by using NCSI incompatible MAC ftgmac100@1e66, we thought this patch is still having value, by providing an extra option to user to be able to use ftgmac100@1e66 for NCSI, and this is also true for AST2500. --Hongwei > > -Original Message- > > From: Linux-aspeed > > [mailto:linux-aspeed-bounces+dylan_hung=aspeedtech@lists.ozlabs.or > > g] > > On Behalf Of Joel Stanley > > Sent: 2020?12?22? 10:26 AM > > To: Hongwei Zhang ; Ryan Chen > > > > Cc: linux-aspeed ; netdev > > ; OpenBMC Maillist ; > > Linux Kernel Mailing List ; Jakub > > Kicinski ; David S Miller > > Subject: Re: [Aspeed, ncsi-rx, v1 0/1] net: ftgmac100: Fix AST2600EVB > > NCSI RX issue > > > > On Mon, 21 Dec 2020 at 17:01, Hongwei Zhang wrote: > > > > > > Dear Reviewer, > > > > > > When FTGMAC100 driver is used on other NCSI Ethernet controllers, > > > few controllers have compatible issue. One example is Intel I210 > > > Ethernet controller on AST2600 BMC, with FTGMAC100 driver, it always > > > trigger RXDES0_RX_ERR error, cause NCSI initialization failure, > > > removing FTGMAC100_RXDES0_RX_ERR bit from RXDES0_ANY_ERROR fix the issue. > > > > I work with a few systems that use the i210 on the 2600. We haven't > > seen this issue in our testing. > > > > Is there something specific about the setup that you use to trigger this? > > > > Ryan, is this an issue that Aspeed is aware of? > > > > Cheers, > > > > Joel Hello Joel, Thanks for your review, please see my response to Dylan, he pointed out the root cause of the issue. -- Hongwei > > > > > > > > Here are part of the debug logs: > > > .. > > > [ 35.075552] ftgmac100_hard_start_xmit TXDESO=b03c > > > [ 35.080843] ftgmac100 1e66.ethernet eth0: tx_complete_packet 55 > > > [ 35.087141] ftgmac100 1e66.ethernet eth0: rx_packet_error > > RXDES0=0xb0070040 > > > [ 37.067831] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready > > > > > > > > > This patch add a configurable flag, FTGMAC100_RXDES0_RX_ERR_CHK, in > > > FTGMAC100 driver, it is YES by default, so keep the orignal define > > > of RXDES0_ANY_ERROR. If it is needed, user can set the flag to NO to > > > remove the RXDES0_RX_ERR bit, to fix the issue. > > > > > > Hongwei Zhang (1): > > > net: ftgmac100: Fix AST2600 EVB NCSI RX issue > > > > > > drivers/net/ethernet/faraday/Kconfig | 9 + > > > drivers/net/ethernet/faraday/ftgmac100.h | 8 > > > 2 files changed, 17 insertions(+) > > > > > > -- > > > 2.17.1 > > > >
[Aspeed, v1 1/1] net: ftgmac100: Change the order of getting MAC address
> From: Jakub Kicinski
> Sent: Monday, December 28, 2020 5:01 PM
>
> On Tue, 22 Dec 2020 22:00:34 +0100 Andrew Lunn wrote:
> > On Tue, Dec 22, 2020 at 09:46:52PM +0100, Heiner Kallweit wrote:
> > > On 22.12.2020 21:14, Hongwei Zhang wrote:
> > > > Dear Reviewer,
> > > >
> > > > Use native MAC address is preferred over other choices, thus change the
> > > > order
> > > > of reading MAC address, try to read it from MAC chip first, if it's not
> > > > availabe, then try to read it from device tree.
> > > >
> > > > Hi Heiner,
> > > >
> > > >> From: Heiner Kallweit
> > > >> Sent: Monday, December 21, 2020 4:37 PM
> > > >>> Change the order of reading MAC address, try to read it from MAC chip
> > > >>> first, if it's not availabe, then try to read it from device tree.
> > > >>>
> > > >> This commit message leaves a number of questions. It seems the change
> > > >> isn't related at all to the
> > > >> change that it's supposed to fix.
> > > >>
> > > >> - What is the issue that you're trying to fix?
> > > >> - And what is wrong with the original change?
> > > >
> > > > There is no bug or something wrong with the original code. This patch
> > > > is for
> > > > improving the code. We thought if the native MAC address is available,
> > > > then
> > > > it's preferred over MAC address from dts (assuming both sources are
> > > > available).
> > > >
> > > > One possible scenario, a MAC address is set in dts and the BMC image is
> > > > compiled and loaded into more than one platform, then the platforms will
> > > > have network issue due to the same MAC address they read.
> > > >
> > >
> > > Typically the DTS MAC address is overwritten by the boot loader, e.g.
> > > uboot.
> > > And the boot loader can read it from chip registers. There are more
> > > drivers
> > > trying to read the MAC address from DTS first. Eventually, I think, the
> > > code
> > > here will read the same MAC address from chip registers as uboot did
> > > before.
Thanks for your review, Heiner,
I am working on a platform and want to use the method you said, reading from DTS
is easy, but overwrite the MAC in DTS with chip MAC address, it will change the
checksum of the image. Would you please provide an implementation example?
Thanks!
> >
> > Do we need to worry about, the chip contains random junk, which passes
> > the validitiy test? Before this patch the value from DT would be used,
> > and the random junk is ignored. Is this change possibly going to cause
> > a regression?
Hi Andrew,
Thanks for your review. Yes, yours is a good point, as my change relies on
the driver's ability to read correct MAC from the chip, or the check of
is_valid_ether_addr(), which only checking for zeros and multicasting MAC.
On the other hand, your concern is still true if no MAC is defined in DTS
file.
Thanks!
>
> Hongwei, please address Andrew's questions.
>
> Once the discussion is over please repost the patches as
> git-format-patch would generate them. The patch 2/2 of this
> series is not really a patch, which confuses all patch handling
> systems.
>
> It also appears that 35c54922dc97 ("ARM: dts: tacoma: Add reserved
> memory for ramoops") does not exist upstream.
>
Hi Jakub,
Thanks for your review; I am quite new to the contribution process. I will
resubmit my
patch with the SHA value issue fixed. Please see my response at above.
--Hongwei
--
2.17.1
[Aspeed, v2 2/2] net: ftgmac100: Change the order of getting MAC address
Dear Reviewer,
Use native MAC address is preferred over other choices, thus change the order
of reading MAC address, try to read it from MAC chip first, if it's not
availabe, then try to read it from device tree.
Hi Heiner,
> From: Heiner Kallweit
> Sent: Monday, December 21, 2020 4:37 PM
> > Change the order of reading MAC address, try to read it from MAC chip
> > first, if it's not availabe, then try to read it from device tree.
> >
> This commit message leaves a number of questions. It seems the change isn't
> related at all to the
> change that it's supposed to fix.
>
> - What is the issue that you're trying to fix?
> - And what is wrong with the original change?
There is no bug or something wrong with the original code. This patch is for
improving the code. We thought if the native MAC address is available, then
it's preferred over MAC address from dts (assuming both sources are available).
One possible scenario, a MAC address is set in dts and the BMC image is
compiled and loaded into more than one platform, then the platforms will
have network issue due to the same MAC address they read.
Thanks for your review, I've update the patch to fix the comments.
>
> > Fixes: 35c54922dc97 ("ARM: dts: tacoma: Add reserved memory for
> > ramoops")
> > Signed-off-by: Hongwei Zhang
> > ---
> > drivers/net/ethernet/faraday/ftgmac100.c | 22 +-
> > 1 file changed, 13 insertions(+), 9 deletions(-)
--Hongwei
[Aspeed, v2 0/2] net: ftgmac100: Change the order of getting MAC address
Dear Reviewer, Use native MAC address is preferred over other choices, thus change the order of reading MAC address, try to read it from MAC chip first, if it's not availabe, then try to read it from device tree. Thanks, --Hongwei Changelog: v2: - Corrected comments in the patch v1: https://patchwork.ozlabs.org/project/linux-aspeed/list/?series=221576 - Initial submission Hongwei Zhang (1): net: ftgmac100: Change the order of getting MAC address drivers/net/ethernet/faraday/ftgmac100.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) -- 2.17.1
[Aspeed, v2 1/2] net: ftgmac100: Change the order of getting MAC address
Change the order of reading MAC address, try to read it from MAC chip
first, if it's not availabe, then try to read it from device tree.
Fixes: 35c54922dc97 ("ARM: dts: tacoma: Add reserved memory for ramoops")
Signed-off-by: Hongwei Zhang
---
drivers/net/ethernet/faraday/ftgmac100.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 65cd25372020..713e9325bef8 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -184,14 +184,7 @@ static void ftgmac100_initial_mac(struct ftgmac100 *priv)
unsigned int l;
void *addr;
- addr = device_get_mac_address(priv->dev, mac, ETH_ALEN);
- if (addr) {
- ether_addr_copy(priv->netdev->dev_addr, mac);
- dev_info(priv->dev, "Read MAC address %pM from device tree\n",
-mac);
- return;
- }
-
+ /* Try to read MAC from chip first */
m = ioread32(priv->base + FTGMAC100_OFFSET_MAC_MADR);
l = ioread32(priv->base + FTGMAC100_OFFSET_MAC_LADR);
@@ -205,7 +198,18 @@ static void ftgmac100_initial_mac(struct ftgmac100 *priv)
if (is_valid_ether_addr(mac)) {
ether_addr_copy(priv->netdev->dev_addr, mac);
dev_info(priv->dev, "Read MAC address %pM from chip\n", mac);
- } else {
+ return;
+ }
+
+ /* Get MAC from device tree if it cannot be read from the chip */
+ addr = device_get_mac_address(priv->dev, mac, ETH_ALEN);
+ if (addr) {
+ ether_addr_copy(priv->netdev->dev_addr, mac);
+ dev_info(priv->dev, "Read MAC address %pM from device tree\n",
+ mac);
+ return;
+ }
+ else {
eth_hw_addr_random(priv->netdev);
dev_info(priv->dev, "Generated random MAC address %pM\n",
priv->netdev->dev_addr);
--
2.17.1
[Aspeed, v1 1/1] net: ftgmac100: Change the order of getting MAC address
Change the order of reading MAC address, try to read it from MAC chip
first, if it's not availabe, then try to read it from device tree.
Fixes: 35c54922dc97 ("ARM: dts: tacoma: Add reserved memory for ramoops")
Signed-off-by: Hongwei Zhang
---
drivers/net/ethernet/faraday/ftgmac100.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 65cd25372020..9be69cbdab96 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -184,14 +184,7 @@ static void ftgmac100_initial_mac(struct ftgmac100 *priv)
unsigned int l;
void *addr;
- addr = device_get_mac_address(priv->dev, mac, ETH_ALEN);
- if (addr) {
- ether_addr_copy(priv->netdev->dev_addr, mac);
- dev_info(priv->dev, "Read MAC address %pM from device tree\n",
-mac);
- return;
- }
-
+ /* Read from Chip if not from chip */
m = ioread32(priv->base + FTGMAC100_OFFSET_MAC_MADR);
l = ioread32(priv->base + FTGMAC100_OFFSET_MAC_LADR);
@@ -205,7 +198,18 @@ static void ftgmac100_initial_mac(struct ftgmac100 *priv)
if (is_valid_ether_addr(mac)) {
ether_addr_copy(priv->netdev->dev_addr, mac);
dev_info(priv->dev, "Read MAC address %pM from chip\n", mac);
- } else {
+ return;
+ }
+
+ /* Read from Chip if not from device tree */
+ addr = device_get_mac_address(priv->dev, mac, ETH_ALEN);
+ if (addr) {
+ ether_addr_copy(priv->netdev->dev_addr, mac);
+ dev_info(priv->dev, "Read MAC address %pM from device tree\n",
+ mac);
+ return;
+ }
+ else {
eth_hw_addr_random(priv->netdev);
dev_info(priv->dev, "Generated random MAC address %pM\n",
priv->netdev->dev_addr);
--
2.17.1
[Aspeed, v1 0/1] net: ftgmac100: Change the order of getting MAC address
Dear Reviewer, Use native MAC address is preferred over other choices, thus change the order of reading MAC address, try to read it from MAC chip first, if it's not availabe, then try to read it from device tree. Hongwei Zhang (1): net: ftgmac100: Change the order of getting MAC address drivers/net/ethernet/faraday/ftgmac100.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) -- 2.17.1
[Aspeed,ncsi-rx, v2 1/1] net: ftgmac100: Fix AST2600 EVB NCSI RX issue
When FTGMAC100 driver is used on other NCSI Ethernet controllers, few
controllers have compatible issue, removing FTGMAC100_RXDES0_RX_ERR bit
from RXDES0_ANY_ERROR can fix the issue.
Fixes: 7ee2d5b4d4340353 ("ARM: dts: nuvoton: Add Fii Kudo system")
Signed-off-by: Hongwei Zhang
---
drivers/net/ethernet/faraday/Kconfig | 9 +
drivers/net/ethernet/faraday/ftgmac100.h | 8
2 files changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/faraday/Kconfig
b/drivers/net/ethernet/faraday/Kconfig
index c2677ec0564d..ccd0c30be0db 100644
--- a/drivers/net/ethernet/faraday/Kconfig
+++ b/drivers/net/ethernet/faraday/Kconfig
@@ -38,4 +38,13 @@ config FTGMAC100
from Faraday. It is used on Faraday A369, Andes AG102 and some
other ARM/NDS32 SoC's.
+config FTGMAC100_RXDES0_RX_ERR_CHK
+ bool "Include FTGMAC100_RXDES0_RX_ERR in RXDES0_ANY_ERROR"
+ default y
+ depends on FTGMAC100
+ help
+ Say N here if the NCSI controller on your platform has compatible
+ issue with FTGMAC100, thus always trigger RXDES0_RX_ERR. Exclude
+ this bit can fix the issue.
+
endif # NET_VENDOR_FARADAY
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h
b/drivers/net/ethernet/faraday/ftgmac100.h
index 63b3e02fab16..59e1bd52d261 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -251,12 +251,20 @@ struct ftgmac100_rxdes {
#define FTGMAC100_RXDES0_RXPKT_RDY (1 << 31)
/* Errors we care about for dropping packets */
+#ifdef CONFIG_FTGMAC100_RXDES0_RX_ERR_CHK
#define RXDES0_ANY_ERROR ( \
FTGMAC100_RXDES0_RX_ERR | \
FTGMAC100_RXDES0_CRC_ERR| \
FTGMAC100_RXDES0_FTL| \
FTGMAC100_RXDES0_RUNT | \
FTGMAC100_RXDES0_RX_ODD_NB)
+#else
+#define RXDES0_ANY_ERROR ( \
+ FTGMAC100_RXDES0_CRC_ERR| \
+ FTGMAC100_RXDES0_FTL| \
+ FTGMAC100_RXDES0_RUNT | \
+ FTGMAC100_RXDES0_RX_ODD_NB)
+#endif
#define FTGMAC100_RXDES1_VLANTAG_CI0x
#define FTGMAC100_RXDES1_PROT_MASK (0x3 << 20)
--
2.17.1
[Aspeed,ncsi-rx, v2 0/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
Dear Reviewer, When FTGMAC100 driver is used on other NCSI Ethernet controllers, few controllers have compatible issue. One example is Intel I210 Ethernet controller on AST2600 BMC, with FTGMAC100 driver, it always trigger RXDES0_RX_ERR error, cause NCSI initialization failure, removing FTGMAC100_RXDES0_RX_ERR bit from RXDES0_ANY_ERROR fix the issue. Here are part of the debug logs: .. [ 35.075552] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.080843] ftgmac100 1e66.ethernet eth0: tx_complete_packet 55 [ 35.087141] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.094448] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.101498] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.108205] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 35.287808] i2c i2c-1: new_device: Instantiated device slave-mqueue at 0x12 [ 35.428379] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.433624] ftgmac100 1e66.ethernet eth0: tx_complete_packet 56 [ 35.439915] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.447225] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.454273] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.460972] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 35.797825] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.803241] ftgmac100 1e66.ethernet eth0: tx_complete_packet 57 [ 35.809541] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.816848] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.823899] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.830597] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 36.179914] ftgmac100_hard_start_xmit TXDESO=b03c [ 36.185160] ftgmac100 1e66.ethernet eth0: tx_complete_packet 58 [ 36.191454] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 36.198761] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 36.205813] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 36.212513] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 36.593688] ftgmac100_hard_start_xmit TXDESO=b03c [ 36.602937] ftgmac100 1e66.ethernet eth0: tx_complete_packet 59 [ 36.609244] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 36.616558] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 36.623608] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 36.630315] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 37.031524] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [ 37.067831] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready This patch add a configurable flag, FTGMAC100_RXDES0_RX_ERR_CHK, in FTGMAC100 driver, it is YES by default, so keep the orignal define of RXDES0_ANY_ERROR. If it is needed, user can set the flag to NO to remove the RXDES0_RX_ERR bit, to fix the issue. Hongwei Zhang (1): net: ftgmac100: Fix AST2600 EVB NCSI RX issue drivers/net/ethernet/faraday/Kconfig | 9 + drivers/net/ethernet/faraday/ftgmac100.h | 8 2 files changed, 17 insertions(+) -- 2.17.1
[Aspeed,ncsi-rx, v1 0/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
Hi Andrew, > From: Andrew Jeffery > > > Fix AST2600 EVB NCSI RX timeout issue by removing > > FTGMAC100_RXDES0_RX_ERR bit from macro RXDES0_ANY_ERROR. > > But why? What is wrong with the EVB that this change resolves? Which revision > of the EVB? > > The change affects all designs using the MAC, not just the AST2600 EVB. Why > is this patch an > appropriate course of action? Can we not add a quirk targeting the specific > board (e.g. a devicetree > property)? You are correct. I was in a rush and didn't put this patch under whole picture of the driver it affected. I revised the cover letter with more information, also use a configurable flag to isolate the change. please review the new approach. > > Andrew > Thanks, --Hongwei
[Aspeed,ncsi-rx, v1] Answer to initial submission
Hi Jakub, > From: Jakub Kicinski > > > ... > > Signed-off-by: Hongwei Zhang > > Thanks for the patch. Please repost CCing the netdev mailing list so it can > be merged to the networking > tree (which I assume is your intent). > Please also include a Fixes tag pointing to the commit where the timeout > issue started (even if it's the > first commit of the driver). > I updated the cc list and cover letter accordingly, also addressed Andrew's question. please review. Thanks, --Hongwei -- 2.17.1
[Aspeed,ncsi-rx, v1 1/1] net: ftgmac100: Fix AST2600 EVB NCSI RX issue
When FTGMAC100 driver is used on other NCSI Ethernet controllers, few
controllers have compatible issue, removing FTGMAC100_RXDES0_RX_ERR bit
from RXDES0_ANY_ERROR can fix the issue.
Fixes: 7ee2d5b4d4340353 ("ARM: dts: nuvoton: Add Fii Kudo system")
Signed-off-by: Hongwei Zhang
---
drivers/net/ethernet/faraday/Kconfig | 9 +
drivers/net/ethernet/faraday/ftgmac100.h | 8
2 files changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/faraday/Kconfig
b/drivers/net/ethernet/faraday/Kconfig
index c2677ec0564d..ccd0c30be0db 100644
--- a/drivers/net/ethernet/faraday/Kconfig
+++ b/drivers/net/ethernet/faraday/Kconfig
@@ -38,4 +38,13 @@ config FTGMAC100
from Faraday. It is used on Faraday A369, Andes AG102 and some
other ARM/NDS32 SoC's.
+config FTGMAC100_RXDES0_RX_ERR_CHK
+ bool "Include FTGMAC100_RXDES0_RX_ERR in RXDES0_ANY_ERROR"
+ default y
+ depends on FTGMAC100
+ help
+ Say N here if the NCSI controller on your platform has compatible
+ issue with FTGMAC100, thus always trigger RXDES0_RX_ERR. Exclude
+ this bit can fix the issue.
+
endif # NET_VENDOR_FARADAY
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h
b/drivers/net/ethernet/faraday/ftgmac100.h
index 63b3e02fab16..59e1bd52d261 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -251,12 +251,20 @@ struct ftgmac100_rxdes {
#define FTGMAC100_RXDES0_RXPKT_RDY (1 << 31)
/* Errors we care about for dropping packets */
+#ifdef CONFIG_FTGMAC100_RXDES0_RX_ERR_CHK
#define RXDES0_ANY_ERROR ( \
FTGMAC100_RXDES0_RX_ERR | \
FTGMAC100_RXDES0_CRC_ERR| \
FTGMAC100_RXDES0_FTL| \
FTGMAC100_RXDES0_RUNT | \
FTGMAC100_RXDES0_RX_ODD_NB)
+#else
+#define RXDES0_ANY_ERROR ( \
+ FTGMAC100_RXDES0_CRC_ERR| \
+ FTGMAC100_RXDES0_FTL| \
+ FTGMAC100_RXDES0_RUNT | \
+ FTGMAC100_RXDES0_RX_ODD_NB)
+#endif
#define FTGMAC100_RXDES1_VLANTAG_CI0x
#define FTGMAC100_RXDES1_PROT_MASK (0x3 << 20)
--
2.17.1
[Aspeed,ncsi-rx, v1 0/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
Dear Reviewer, When FTGMAC100 driver is used on other NCSI Ethernet controllers, few controllers have compatible issue. One example is Intel I210 Ethernet controller on AST2600 BMC, with FTGMAC100 driver, it always trigger RXDES0_RX_ERR error, cause NCSI initialization failure, removing FTGMAC100_RXDES0_RX_ERR bit from RXDES0_ANY_ERROR fix the issue. Here are part of the debug logs: .. [ 35.075552] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.080843] ftgmac100 1e66.ethernet eth0: tx_complete_packet 55 [ 35.087141] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.094448] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.101498] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.108205] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 35.287808] i2c i2c-1: new_device: Instantiated device slave-mqueue at 0x12 [ 35.428379] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.433624] ftgmac100 1e66.ethernet eth0: tx_complete_packet 56 [ 35.439915] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.447225] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.454273] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.460972] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 35.797825] ftgmac100_hard_start_xmit TXDESO=b03c [ 35.803241] ftgmac100 1e66.ethernet eth0: tx_complete_packet 57 [ 35.809541] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 35.816848] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 35.823899] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 35.830597] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 36.179914] ftgmac100_hard_start_xmit TXDESO=b03c [ 36.185160] ftgmac100 1e66.ethernet eth0: tx_complete_packet 58 [ 36.191454] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 36.198761] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 36.205813] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 36.212513] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 36.593688] ftgmac100_hard_start_xmit TXDESO=b03c [ 36.602937] ftgmac100 1e66.ethernet eth0: tx_complete_packet 59 [ 36.609244] ftgmac100 1e66.ethernet eth0: rx_packet_error RXDES0=0xb0070040 [ 36.616558] ftgmac100_rx_packet RXDES0=b0070040 RXDES1=f080 RXDES2=88f8 [ 36.623608] ftgmac100 1e66.ethernet eth0: rx_packet_error 0xb0070040 [ 36.630315] ftgmac100 1e66.ethernet eth0: [ISR] = 0xb0070040: RX_ERR [ 37.031524] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready [ 37.067831] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready This patch add a configurable flag, FTGMAC100_RXDES0_RX_ERR_CHK, in FTGMAC100 driver, it is YES by default, so keep the orignal define of RXDES0_ANY_ERROR. If it is needed, user can set the flag to NO to remove the RXDES0_RX_ERR bit, to fix the issue. Hongwei Zhang (1): net: ftgmac100: Fix AST2600 EVB NCSI RX issue drivers/net/ethernet/faraday/Kconfig | 9 + drivers/net/ethernet/faraday/ftgmac100.h | 8 2 files changed, 17 insertions(+) -- 2.17.1
[Aspeed,ncsi-rx, v1 1/1] net: ftgmac100: Fix AST2600 EVB NCSI RX issue
Fix AST2600 EVB NCSI RX timeout issue by removing FTGMAC100_RXDES0_RX_ERR bit
from macro RXDES0_ANY_ERROR.
Signed-off-by: Hongwei Zhang
---
drivers/net/ethernet/faraday/ftgmac100.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.h
b/drivers/net/ethernet/faraday/ftgmac100.h
index 63b3e02fab16..734477e6f72f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.h
+++ b/drivers/net/ethernet/faraday/ftgmac100.h
@@ -252,7 +252,6 @@ struct ftgmac100_rxdes {
/* Errors we care about for dropping packets */
#define RXDES0_ANY_ERROR ( \
- FTGMAC100_RXDES0_RX_ERR | \
FTGMAC100_RXDES0_CRC_ERR| \
FTGMAC100_RXDES0_FTL| \
FTGMAC100_RXDES0_RUNT | \
--
2.17.1
[Aspeed,ncsi-rx, v1 0/1] net: ftgmac100: Fix AST2600EVB NCSI RX issue
Dear Reviewer, Fix AST2600 EVB NCSI RX timeout issue by removing FTGMAC100_RXDES0_RX_ERR bit from macro RXDES0_ANY_ERROR. Hongwei Zhang (1): net: ftgmac100: Fix AST2600 EVB NCSI RX issue drivers/net/ethernet/faraday/ftgmac100.h | 1 - 1 file changed, 1 deletion(-) -- 2.17.1
[v2, 2/2] gpio: dts: aspeed: Add SGPIO driver
Thanks Linus, > > I sent a separate patch to fix this up the way I want it with the file named > gpio-aspeed-sgpio.c and > CONFIG_GPIO_ASPEED_SGPIO. > > I don't want to mix up the namespaces of something Aspeed-generic with the > namespace of the GPIO > subsystem. SGPIO is the name of a specific Aspeed component. > > Yours, > Linus Walleij I agree and gpio-aspeed-sgpio.c is better. Regards, --Hongwei
[v2, 1/2] gpio: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
arch/arm/Kconfig | 2 ++
arch/arm/boot/dts/aspeed-g5.dtsi | 16 +++-
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2436021..c9f08ab 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1460,6 +1460,8 @@ config ARCH_NR_GPIO
default 416 if ARCH_SUNXI
default 392 if ARCH_U8500
default 352 if ARCH_VT8500
+ default 312 if MACH_ASPEED_G5
+ default 304 if MACH_ASPEED_G4
default 288 if ARCH_ROCKCHIP
default 264 if MACH_H4700
default 0
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 00f05bd..85da7ea 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -311,7 +311,7 @@
#gpio-cells = <2>;
gpio-controller;
compatible = "aspeed,ast2500-gpio";
- reg = <0x1e78 0x1000>;
+ reg = <0x1e78 0x200>;
interrupts = <20>;
gpio-ranges = < 0 0 232>;
clocks = < ASPEED_CLK_APB>;
@@ -319,6 +319,20 @@
#interrupt-cells = <2>;
};
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_sgpm_default>;
+ };
+
rtc: rtc@1e781000 {
compatible = "aspeed,ast2500-rtc";
reg = <0x1e781000 0x18>;
--
2.7.4
[v2, 2/2] gpio: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC. Signed-off-by: Hongwei Zhang --- drivers/gpio/Kconfig | 8 drivers/gpio/Makefile| 1 + diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index bb13c26..e94f903 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -120,6 +120,14 @@ config GPIO_ASPEED help Say Y here to support Aspeed AST2400 and AST2500 GPIO controllers. +config SGPIO_ASPEED + bool "Aspeed SGPIO support" + depends on (ARCH_ASPEED || COMPILE_TEST) && OF_GPIO + select GPIO_GENERIC + select GPIOLIB_IRQCHIP + help + Say Y here to support Aspeed AST2500 SGPIO functionality. + config GPIO_ATH79 tristate "Atheros AR71XX/AR724X/AR913X GPIO support" default y if ATH79 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index a4e9117..bebbd82 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -32,6 +32,7 @@ obj-$(CONFIG_GPIO_AMD_FCH)+= gpio-amd-fch.o obj-$(CONFIG_GPIO_AMDPT) += gpio-amdpt.o obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o +obj-$(CONFIG_SGPIO_ASPEED) += sgpio-aspeed.o obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o obj-$(CONFIG_GPIO_BCM_KONA)+= gpio-bcm-kona.o obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o -- 2.7.4
[v2, 0/2] gpio: dts: aspeed: Add SGPIO driver
Hello, This short series introduce the Kconfig, Makefile, and dts for the Aspeed AST2500 SGPIO controller. This is the last patch set. Please review. [v2]: changes between v1 and v2: - split the patches based on review feedback. [v1]: Initial commit The related SGPIO driver has been accepted and merged into v5.4: _http://patchwork.ozlabs.org/patch/1150357/ The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Thanks! Hongwei Zhang (1): gpio: dts: aspeed: Add SGPIO driver arch/arm/Kconfig | 2 ++ arch/arm/boot/dts/aspeed-g5.dtsi | 16 +++- drivers/gpio/Kconfig | 8 drivers/gpio/Makefile| 1 + 4 files changed, 26 insertions(+), 1 deletion(-) -- 2.7.4
[v1, 0/1] gpio: dts: aspeed: Add SGPIO driver
> > > The related SGPIO driver has been accepted and merged into v5.4: > > _http://patchwork.ozlabs.org/patch/1150357/ > > Oh what a mess, it didn't add the necessary code into Kconfig and Makefile, > also names it sgpio-gpio.c > when everything else is named gpio-sgpio.c. > > I guess I have to fix it up. My fault for missing. > > Linus Walleij Thanks Linus, It's not your fault, I misunderstood a earlier comment from another reviewer and thought I should wait until the driver is accecpted, and then submit the patch to include / enable it. As Bart suggested, I splitte the patches. Regarding the driver name, following the gpio-SoC_name.o convention in the Makefile, we choose sgpio-aspeed.o . --Hongwei
[v1, 1/1] gpio: dts: aspeed: Add SGPIO driver
> > obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o > > -- > > 2.7.4 > > > > This should be split into separate patches with one extending the binding > document and one adding > actual support. > > Bart Thanks Bart, I just submitted splitted patches. --Hongwei
[v1, 1/1] gpio: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
arch/arm/Kconfig | 2 ++
arch/arm/boot/dts/aspeed-g5.dtsi | 16 +++-
drivers/gpio/Kconfig | 8
drivers/gpio/Makefile| 1 +
4 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2436021..c9f08ab 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1460,6 +1460,8 @@ config ARCH_NR_GPIO
default 416 if ARCH_SUNXI
default 392 if ARCH_U8500
default 352 if ARCH_VT8500
+ default 312 if MACH_ASPEED_G5
+ default 304 if MACH_ASPEED_G4
default 288 if ARCH_ROCKCHIP
default 264 if MACH_H4700
default 0
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 00f05bd..85da7ea 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -311,7 +311,7 @@
#gpio-cells = <2>;
gpio-controller;
compatible = "aspeed,ast2500-gpio";
- reg = <0x1e78 0x1000>;
+ reg = <0x1e78 0x200>;
interrupts = <20>;
gpio-ranges = < 0 0 232>;
clocks = < ASPEED_CLK_APB>;
@@ -319,6 +319,20 @@
#interrupt-cells = <2>;
};
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_sgpm_default>;
+ };
+
rtc: rtc@1e781000 {
compatible = "aspeed,ast2500-rtc";
reg = <0x1e781000 0x18>;
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bb13c26..e94f903 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -120,6 +120,14 @@ config GPIO_ASPEED
help
Say Y here to support Aspeed AST2400 and AST2500 GPIO controllers.
+config SGPIO_ASPEED
+ bool "Aspeed SGPIO support"
+ depends on (ARCH_ASPEED || COMPILE_TEST) && OF_GPIO
+ select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
+ help
+ Say Y here to support Aspeed AST2500 SGPIO functionality.
+
config GPIO_ATH79
tristate "Atheros AR71XX/AR724X/AR913X GPIO support"
default y if ATH79
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a4e9117..bebbd82 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_GPIO_AMD_FCH)+= gpio-amd-fch.o
obj-$(CONFIG_GPIO_AMDPT) += gpio-amdpt.o
obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o
obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o
+obj-$(CONFIG_SGPIO_ASPEED) += sgpio-aspeed.o
obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o
obj-$(CONFIG_GPIO_BCM_KONA)+= gpio-bcm-kona.o
obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o
--
2.7.4
[v1, 0/1] gpio: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
arch/arm/Kconfig | 2 ++
arch/arm/boot/dts/aspeed-g5.dtsi | 16 +++-
drivers/gpio/Kconfig | 8
drivers/gpio/Makefile| 1 +
4 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2436021..c9f08ab 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1460,6 +1460,8 @@ config ARCH_NR_GPIO
default 416 if ARCH_SUNXI
default 392 if ARCH_U8500
default 352 if ARCH_VT8500
+ default 312 if MACH_ASPEED_G5
+ default 304 if MACH_ASPEED_G4
default 288 if ARCH_ROCKCHIP
default 264 if MACH_H4700
default 0
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 00f05bd..85da7ea 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -311,7 +311,7 @@
#gpio-cells = <2>;
gpio-controller;
compatible = "aspeed,ast2500-gpio";
- reg = <0x1e78 0x1000>;
+ reg = <0x1e78 0x200>;
interrupts = <20>;
gpio-ranges = < 0 0 232>;
clocks = < ASPEED_CLK_APB>;
@@ -319,6 +319,20 @@
#interrupt-cells = <2>;
};
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_sgpm_default>;
+ };
+
rtc: rtc@1e781000 {
compatible = "aspeed,ast2500-rtc";
reg = <0x1e781000 0x18>;
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bb13c26..e94f903 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -120,6 +120,14 @@ config GPIO_ASPEED
help
Say Y here to support Aspeed AST2400 and AST2500 GPIO controllers.
+config SGPIO_ASPEED
+ bool "Aspeed SGPIO support"
+ depends on (ARCH_ASPEED || COMPILE_TEST) && OF_GPIO
+ select GPIO_GENERIC
+ select GPIOLIB_IRQCHIP
+ help
+ Say Y here to support Aspeed AST2500 SGPIO functionality.
+
config GPIO_ATH79
tristate "Atheros AR71XX/AR724X/AR913X GPIO support"
default y if ATH79
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a4e9117..bebbd82 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_GPIO_AMD_FCH)+= gpio-amd-fch.o
obj-$(CONFIG_GPIO_AMDPT) += gpio-amdpt.o
obj-$(CONFIG_GPIO_ARIZONA) += gpio-arizona.o
obj-$(CONFIG_GPIO_ASPEED) += gpio-aspeed.o
+obj-$(CONFIG_SGPIO_ASPEED) += sgpio-aspeed.o
obj-$(CONFIG_GPIO_ATH79) += gpio-ath79.o
obj-$(CONFIG_GPIO_BCM_KONA)+= gpio-bcm-kona.o
obj-$(CONFIG_GPIO_BD70528) += gpio-bd70528.o
--
2.7.4
[v1, 0/1] gpio: dts: aspeed: Add SGPIO driver
Hello, This short series introduce the Kconfig, Makefile, and dts for the Aspeed AST2500 SGPIO controller. This is the last patch set. Please review. [v1]: Initial commit The related SGPIO driver has been accepted and merged into v5.4: _http://patchwork.ozlabs.org/patch/1150357/ The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Thanks! Hongwei Zhang (1): gpio: dts: aspeed: Add SGPIO driver arch/arm/Kconfig | 2 ++ arch/arm/boot/dts/aspeed-g5.dtsi | 16 +++- drivers/gpio/Kconfig | 8 drivers/gpio/Makefile| 1 + 4 files changed, 26 insertions(+), 1 deletion(-) -- 2.7.4
[v7 2/2] gpio: aspeed: Add SGPIO driver
Hello Linus,
Thanks for your review! I just submitted v8 to the list, please help to review
it again.
Since you have already merged the dt-binding document [v7 1/2], and I don't
have your
update to this file, so to avoid confusion, I only include the driver code in
v8.
Regards,
--Hongwei
P.S. sorry previous response used wrong In-Reply-To ID, so resent it again.
> From: Linus Walleij
> Sent: Wednesday, August 14, 2019 4:09 AM
> To: Hongwei Zhang
> Cc: Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley; linux-aspeed;
> Bartosz Golaszewski;
> linux-kernel@vger.kernel.org; Linux ARM
> Subject: Re: [v7 2/2] gpio: aspeed: Add SGPIO driver
>
> Hi Hongwei,
>
> thanks for your patch!
>
> I have now merged the bindings so you only need to respin this patch.
>
> On Wed, Jul 31, 2019 at 10:02 PM Hongwei Zhang wrote:
>
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > Reviewed-by: Andrew Jeffery
>
> I guess I need to go with this, there are some minor things I still want to
> be fixed:
>
> > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > +offset, int val)
>
> I don't like __underscore_functions because their semantic is ambiguous.
>
done, please see v8.
> Rename this something like aspeed_sgpio_commit() or whatever best fits the
> actual use.
>
> > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> > + struct platform_device *pdev) {
> (...)
> > + rc = gpiochip_irqchip_add(>chip, _sgpio_irqchip,
> > + 0, handle_bad_irq, IRQ_TYPE_NONE);
> (...)
> > + gpiochip_set_chained_irqchip(>chip, _sgpio_irqchip,
> > +gpio->irq,
> > + aspeed_sgpio_irq_handler);
>
> We do not set up chained irqchips like this anymore, sorry.
>
> I am currently rewriting all existing chained drivers to pass an initialized
> irqchip when registering the
> whole gpio chip.
> See drivers/gpio/TODO.
>
> Here are examples:
> https://lore.kernel.org/linux-gpio/20190811080539.15647-1-linus.wall...@linaro.org/
> https://lore.kernel.org/linux-gpio/20190812132554.18313-1-linus.wall...@linaro.org/
>
done, please see v8.
> > + /* set all SGPIO pins as input (1). */
> > + memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in));
>
> Do the irqchip set-up here, before adding the gpio_chip.
>
> > + rc = devm_gpiochip_add_data(>dev, >chip, gpio);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return aspeed_sgpio_setup_irqs(gpio, pdev);
>
> Yours,
> Linus Walleij
[v7 2/2] gpio: aspeed: Add SGPIO driver
Hello Linus,
Thanks for your review! I just submitted v8 to the list, please help to review
it again.
Since you have already merged the dt-binding document [v7 1/2], and I don't
have your
update to this file, so to avoid confusion, I only include the driver code in
v8.
Regards,
--Hongwei
> From: Linus Walleij
> Sent: Wednesday, August 14, 2019 4:09 AM
> To: Hongwei Zhang
> Cc: Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley; linux-aspeed;
> Bartosz Golaszewski;
> linux-kernel@vger.kernel.org; Linux ARM
> Subject: Re: [v7 2/2] gpio: aspeed: Add SGPIO driver
>
> Hi Hongwei,
>
> thanks for your patch!
>
> I have now merged the bindings so you only need to respin this patch.
>
> On Wed, Jul 31, 2019 at 10:02 PM Hongwei Zhang wrote:
>
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > Reviewed-by: Andrew Jeffery
>
> I guess I need to go with this, there are some minor things I still want to
> be fixed:
>
> > +static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > +offset, int val)
>
> I don't like __underscore_functions because their semantic is ambiguous.
>
done, please see v8.
> Rename this something like aspeed_sgpio_commit() or whatever best fits the
> actual use.
>
> > +static int aspeed_sgpio_setup_irqs(struct aspeed_sgpio *gpio,
> > + struct platform_device *pdev) {
> (...)
> > + rc = gpiochip_irqchip_add(>chip, _sgpio_irqchip,
> > + 0, handle_bad_irq, IRQ_TYPE_NONE);
> (...)
> > + gpiochip_set_chained_irqchip(>chip, _sgpio_irqchip,
> > +gpio->irq,
> > + aspeed_sgpio_irq_handler);
>
> We do not set up chained irqchips like this anymore, sorry.
>
> I am currently rewriting all existing chained drivers to pass an initialized
> irqchip when registering the
> whole gpio chip.
> See drivers/gpio/TODO.
>
> Here are examples:
> https://lore.kernel.org/linux-gpio/20190811080539.15647-1-linus.wall...@linaro.org/
> https://lore.kernel.org/linux-gpio/20190812132554.18313-1-linus.wall...@linaro.org/
>
done, please see v8.
> > + /* set all SGPIO pins as input (1). */
> > + memset(gpio->dir_in, 0xff, sizeof(gpio->dir_in));
>
> Do the irqchip set-up here, before adding the gpio_chip.
>
> > + rc = devm_gpiochip_add_data(>dev, >chip, gpio);
> > + if (rc < 0)
> > + return rc;
> > +
> > + return aspeed_sgpio_setup_irqs(gpio, pdev);
>
> Yours,
> Linus Walleij
[v8] gpio: aspeed: Add SGPIO driver
Hello, This short series introduce dt-binding document and a driver for the Aspeed AST2500 SGPIO controller. Please review. [v8]: Changes between v7 and v8: - v7 updates based on Linus' feedback - since Linus has already merged sgpio-aspeed.txt, I only include the driver here to avoid confusion. [v7]: Changes between v6 and v7: - fix missing variable 'reg' assign issue in aspeed_sgpio_set() - v6 feedback updates [v6]: Changes between v5 and v6: - fix a bug in aspeed_sgpio_dir_out() - v5 feedback updates, some comments cleanup The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Hongwei Zhang (1): gpio: aspeed: Add SGPIO driver drivers/gpio/sgpio-aspeed.c | 533 1 file changed, 533 insertions(+) create mode 100644 drivers/gpio/sgpio-aspeed.c -- 2.7.4
[v8 1/1] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
Reviewed-by: Andrew Jeffery
---
drivers/gpio/sgpio-aspeed.c | 533
1 file changed, 533 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..7e99860
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,533 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void sgpio_set_value(struct gpio_chip *gc, unsigned int offset, int val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ void __iomem *addr;
+ u32 reg = 0;
+
+ addr = bank_reg(gpio
[v6 2/2] gpio: aspeed: Add SGPIO driver
Hello Andrew,
Thanks so much for your help.
> From: Andrew Jeffery
> Sent: Tuesday, July 30, 2019 8:19 PM
> To: Hongwei Zhang; Linus Walleij; linux-g...@vger.kernel.org
> Cc: Joel Stanley; linux-asp...@lists.ozlabs.org; Bartosz Golaszewski;
> linux-kernel@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org
> Subject: Re: [v6 2/2] gpio: aspeed: Add SGPIO driver
>
>
>
> On Wed, 31 Jul 2019, at 00:55, Hongwei Zhang wrote:
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > drivers/gpio/sgpio-aspeed.c | 521
> >
> > 1 file changed, 521 insertions(+)
> > create mode 100644 drivers/gpio/sgpio-aspeed.c
> >
> > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > new file mode 100644 index 000..9a17b1a
> > --- /dev/null
> > +++ b/drivers/gpio/sgpio-aspeed.c
> > @@ -0,0 +1,521 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2019 American Megatrends International LLC.
> > + *
> > + * Author: Karthikeyan Mani */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int
> > offset, int val)
> > +{
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + const struct aspeed_sgpio_bank *bank = to_bank(offset);
> > + unsigned long flags;
> > + void __iomem *addr;
> > + u32 reg = 0;
> > +
> > + spin_lock_irqsave(>lock, flags);
> > +
> > + addr = bank_reg(gpio, bank, reg_val);
> > +
> > + if (val)
> > + reg |= GPIO_BIT(offset);
> > + else
> > + reg &= ~GPIO_BIT(offset);
>
> reg is zero-initialised above and you haven't read from addr to assign to
> reg, so the else branch is
> redundant (reg is already zeroed). This path has a bug - you're clearing the
> state of all GPIOs associated
> with addr rather than just the GPIO associated with offset.
>
you're correct, this is fixed in v7.
> > +
> > + iowrite32(reg, addr);
> > +
> > + spin_unlock_irqrestore(>lock, flags); }
> > +
> > +
> > +static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int
> > offset, int val)
> > +{
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(>lock, flags);
> > + gpio->dir_in[GPIO_BANK(offset)] &= ~GPIO_BIT(offset);
> > + spin_unlock_irqrestore(>lock, flags);
> > +
> > + aspeed_sgpio_set(gc, offset, val);
>
> In this case you should probably have an unlocked variant of
> aspeed_sgpio_set() so you can call it inside
> the the critical section above instead of needing to acquire/release the lock
> twice (once above and again
> in aspeed_sgpio_set() as it stands).
>
moved _sgpio_set() so only one pair of acquire/release lock used.
> Cheers,
>
> Andrew
>
Thanks,
--Hongwei
> > +
> > + return 0;
> > +}
> > +
> > --
> > 2.7.4
> >
> >
[v7 2/2] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
Reviewed-by: Andrew Jeffery
---
drivers/gpio/sgpio-aspeed.c | 530
1 file changed, 530 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..752efea
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,530 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ void __iomem *addr;
+ u32 reg = 0;
+
+ addr = bank_reg(gpio
[v7 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
Reviewed-by: Andrew Jeffery
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..8545bbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,55 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
+featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+support the following options:
+- Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+- Support reset tolerance option for each output port
+- Directly connected to APB bus and its shift clock is from APB bus clock
+ divided by a programmable value.
+- Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+
+Required properties:
+
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- ngpios : number of GPIO pins to serialise.
+ (should be multiple of 8, up to 80 pins)
+
+- clocks: A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/gpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ };
--
2.7.4
[v7 0/2] gpio: aspeed: Add SGPIO driver
Hello, This short series introduce dt-binding document and a driver for the Aspeed AST2500 SGPIO controller. Please review. [v7]: Changes between v6 and v7: - fix missing variable 'reg' assign issue in aspeed_sgpio_set() - v6 feedback updates [v6]: Changes between v5 and v6: - fix a bug in aspeed_sgpio_dir_out() - v5 feedback updates, some comments cleanup The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Hongwei Zhang (2): dt-bindings: gpio: aspeed: Add SGPIO support gpio: aspeed: Add SGPIO driver .../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 +++ drivers/gpio/sgpio-aspeed.c| 530 + 2 files changed, 585 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt create mode 100644 drivers/gpio/sgpio-aspeed.c -- 2.7.4
[v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
Hello Linus and Andrew, Thanks for your detailed comments, I just submitted v6 of our update: _http://patchwork.ozlabs.org/cover/1139035/ _http://patchwork.ozlabs.org/patch/1139038/ _http://patchwork.ozlabs.org/patch/1139040/ please ignore my previous patches sent on 07/28, they does not have proper serial title and one of the patch is missing. --Hongwei > From: Linus Walleij > Sent: Monday, July 29, 2019 5:57 PM > To: Andrew Jeffery > Cc: Hongwei Zhang; open list:GPIO SUBSYSTEM; Joel Stanley; open list:OPEN > FIRMWARE AND > FLATTENED DEVICE TREE BINDINGS; linux-aspeed; Bartosz Golaszewski; Rob > Herring; Mark Rutland; > linux-kernel@vger.kernel.org; Linux ARM > Subject: Re: [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support > > On Mon, Jul 29, 2019 at 2:19 AM Andrew Jeffery wrote: > > > The behaviour is to periodically emit the state of all enabled GPIOs > > (i.e. the ngpios value), one per bus clock cycle. There's no explicit > > addressing scheme, the protocol encodes the value for a given GPIO by > > its position in the data stream relative to a pulse on the "load data" > > (LD) line, whose envelope covers the clock cycle for the last GPIO in > > the sequence. Similar to SPI the bus has both out and in lines, which > > cater to output/input GPIOs. > > > > A rough timing diagram for a 16-GPIO configuration looks like what > > I've pasted here: > > > > https://gist.github.com/amboar/c9543af1957854474b8c05ab357f0675 > > OK that is complex. I agree we need to keep this driver together. > > Yours, > Linus Walleij
[v6 2/2] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 521
1 file changed, 521 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..9a17b1a
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,521 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 reg = 0;
+
+ spin_lock_irqsave(>lock, flags
[v6 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..f9ed438
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,55 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
+featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+support the following options:
+- Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+- Support reset tolerance option for each output port
+- Directly connected to APB bus and its shift clock is from APB bus clock
+ divided by a programmable value.
+- Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+
+Required properties:
+
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- ngpios : number of GPIO pins to serialise.
+ (should be multiple of 8, up to 80 pins)
+
+- clocks: A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ };
--
2.7.4
[v6 0/2] gpio: aspeed: Add SGPIO driver
Hello, This short series introduce dt-binding document and a driver for the Aspeed AST2500 SGPIO controller. Please review. [v6]: Changes between v5 and v6: - fix a bug in aspeed_sgpio_dir_out() - v5 feedback updates, some comments cleanup The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Hongwei Zhang (2): dt-bindings: gpio: aspeed: Add SGPIO support gpio: aspeed: Add SGPIO driver .../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 +++ drivers/gpio/sgpio-aspeed.c| 521 + 2 files changed, 576 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt create mode 100644 drivers/gpio/sgpio-aspeed.c -- 2.7.4
[v5 2/2] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 521
1 file changed, 521 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..9a17b1a
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,521 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 reg = 0;
+
+ spin_lock_irqsave(>lock, flags
[v6 2/2] gpio: aspeed: Add SGPIO driver
Hello Linus, Thanks for your detailed comments. We just submitted a v6 of sgpio-aspeed.c, it includes the updates based on your initial feedback: 1. fix a bug in aspeed_sgpio_dir_out() 2. some comments clean up. Regards, --Hongwei > From: Linus Walleij > Sent: Sunday, July 28, 2019 7:38 PM > To: Hongwei Zhang > Cc: Andrew Jeffery; linux-gpio; Joel Stanley; linux-aspeed; Bartosz > Golaszewski; linux-kernel; linux- > arm-kernel > Subject: Re: [v5 2/2] gpio: aspeed: Add SGPIO driver > > On Mon, Jul 22, 2019 at 10:37 PM Hongwei Zhang wrote: > > > As you suspected it correctly, AST2500 utilizes all the 32 bits of the > > registers (data value, interrupt, etc...), such that using 8-bit bands > > [7:0]/[15:8]/23:16]/[31:24] of GPIO_200H for SGPIO_A/B/C/D . > > so registering 10 gpiochip drivers separately will make code more > > complicated, for example gpio_200 register (data_value reg) has to be > > shared by 4 gpiochip instances, and the same is true for gpio204 > > (interrupt reg), and other more registers. > > So we would prefer to keeping current implementation. > > OK this is a pretty good argument. My review assumed one 32-bit register was > not shared between > banks but it is, I see. > > The above situation can be managed by regmap, but that will just a different > complexity so go with this > approach then. > > Yours, > Linus Walleij
[v6 2/2] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 521
1 file changed, 521 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..9a17b1a
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,521 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+static void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 reg = 0;
+
+ spin_lock_irqsave(>lock, flags
[v5 2/2] gpio: aspeed: Add SGPIO driver
Hello Linus,
Thanks for your reviewing, please find my inline comment on why we group the
("A", "B", "C", "D") etc. together at below. We will address other concerns
separately.
--Hongwei
> From: Linus Walleij
> Sent: Saturday, July 20, 2019 3:37 AM
> To: Hongwei Zhang
> Cc: Andrew Jeffery; open list:GPIO SUBSYSTEM; Joel Stanley;
> linux-asp...@lists.ozlabs.org; Bartosz
> Golaszewski; linux-kernel@vger.kernel.org; Linux ARM
> Subject: Re: [v5 2/2] gpio: aspeed: Add SGPIO driver
>
> Hi Hongwei,
>
> thanks for your patch!
>
> some comments and nitpicking below:
>
> On Fri, Jul 19, 2019 at 9:25 PM Hongwei Zhang wrote:
>
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
>
> > +// SPDX-License-Identifier: GPL-2.0+
>
> I think the SPDX people prefer GPL-2.0-or-later
>
> > +#include
>
> Do not include this header in any new code using or providing GPIOs.
>
> > +#include
>
> This should be enough.
>
> > +/*
> > + * Note: The "value" register returns the input value when the GPIO is
> > + * configured as an input.
> > + *
> > + * The "rdata" register returns the output value when the GPIO is
> > + * configured as an output.
> > + */
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > + {
> > + .val_regs = 0x,
> > + .rdata_reg = 0x0070,
> > + .irq_regs = 0x0004,
> > + .names = { "A", "B", "C", "D" },
> > + },
> > + {
> > + .val_regs = 0x001C,
> > + .rdata_reg = 0x0074,
> > + .irq_regs = 0x0020,
> > + .names = { "E", "F", "G", "H" },
> > + },
> > + {
> > + .val_regs = 0x0038,
> > + .rdata_reg = 0x0078,
> > + .irq_regs = 0x003C,
> > + .names = { "I", "J" },
> > + },
> > +};
>
> I guess you have been over the reasons why this is one big GPIO chip instead
> of 10 individual gpio_chips?
>
> It is usally better to have the individual chips, because it is easier to
> just cut down the code to handle
> one instance and not having to offset around the different address ranges.
>
> Even if they all have the same clock, the clocks are reference counted so it
> will just be referenced 10
> times at most.
>
> If they share a few common registers it is not good to split it though. So
> there may be a compelling
> argument for keeping them all together.
>
As you suspected it correctly, AST2500 utilizes all the 32 bits of the
registers
(data value, interrupt, etc...), such that using 8-bit bands
[7:0]/[15:8]/23:16]/[31:24] of GPIO_200H for SGPIO_A/B/C/D .
so registering 10 gpiochip drivers separately will make code more
complicated, for example gpio_200 register (data_value reg) has to be
shared by 4 gpiochip instances, and the same is true for gpio204 (interrupt
reg),
and other more registers.
So we would prefer to keeping current implementation.
> > +/* This will be resolved at compile time */
>
> I don't see why that matters.
>
> > +static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
> > +const struct aspeed_sgpio_bank *bank,
> > +const enum aspeed_sgpio_reg reg)
>
> You don't need inline. The compiler will inline it anyway if it see the need
> for it.
>
> The only time we really use inline is in header files, where we want to point
> out that this function will be
> inlined as there is no compiled code in header files.
>
> > +#define GPIO_BANK(x)((x) >> 5)
> > +#define GPIO_OFFSET(x) ((x) & 0x1f)
> > +#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
>
> OK seems fairly standard.
>
> > +static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int
> > +offset) static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned
> > +int offset, int val) static int aspeed_sgpio_dir_in(struct gpio_chip
> > +*gc, unsigned int offset)
>
> These are fairly standard.
>
> > +static int aspeed_sgpio_dir_out(struct gpio_chip *gc, unsigned int
> > +offset, int val) {
> > + struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(>lock, flags);
> > + gpio->dir_in[GPIO_BANK(offset)
gpio: aspeed: Add SGPIO driver
Hello Andrew,
Thanks for reviewing and suggestions.
please see my inline comments. I just submitted v5 with a proper series title,
please help review them.
In AST2500 datasheet, it says:
"Support up to 80 SGPIO input ports and 80 output ports concurrently only at
the cost of 4 control pins".
Would you please give us some hints on how to configure the SGPIO pins so that
we can use them as input and output 'concurrently' ?
Thanks,
--Hongwei
> From: Andrew Jeffery
> Sent: Wednesday, July 17, 2019 9:45 PM
> To: Hongwei Zhang; Bartosz Golaszewski; Joel Stanley; Linus Walleij
> Cc: linux-g...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-asp...@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3 v4] ARM: dts: aspeed: Add SGPIO driver
>
> Hello Hongwei,
>
> The driver is shaping up! I have a few remaining nitpicks below:
>
> The first is the patch subject:
>
> [PATCH 2/3 v4] ARM: dts: aspeed: Add SGPIO driver
>
> I think one of the first iterations of the patch included the devicetree
> changes. It doesn't any more, as
> the devicetree updates should be done in a separate patch, however the
> subject is now inaccurate.
> Disregarding the [PATCH ...] prefix, it should be:
>
> gpio: aspeed: Add SPGIO driver
>
> As for the [PATCH ...] prefix business, both your bindings patch and your
> driver patch are listed as patch
> 2/3, and there's no third patch in the series. `git format-patch` takes care
> of all this for you - I encourage
> you to get `git send-email` working so you can use it to send the patches
> produced from `git format-
> patch` (as they won't have these problems). Further, if you're sending
> multiple patches it's useful to
> include a cover letter as the patches will be properly threaded underneath
> it, which helps keep related
> patches together and provides a place to talk about your patches that isn't
> the commit message.
>
the v5 is submitted with properly combined patches.
> On Thu, 18 Jul 2019, at 07:00, Hongwei Zhang wrote:
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > drivers/gpio/sgpio-aspeed.c | 518
> >
> > 1 file changed, 518 insertions(+)
> > create mode 100644 drivers/gpio/sgpio-aspeed.c
> >
> > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > new file mode 100644 index 000..715052c
> > --- /dev/null
> > +++ b/drivers/gpio/sgpio-aspeed.c
> > @@ -0,0 +1,518 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 American Megatrends International LLC.
> > + *
> > + * Author: Karthikeyan Mani */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> It would be nice if these were sorted alphabetically. Not overly fussed
> though.
>
done.
> > +
> > +#define MAX_NR_SGPIO 80
> > +
> > +#define ASPEED_SGPIO_CTRL 0x54
> > +
> > +#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
> > +#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
> > +#define ASPEED_SGPIO_ENABLEBIT(0)
> > +
> > +struct aspeed_sgpio {
> > + struct gpio_chip chip;
> > + struct clk *pclk;
> > + spinlock_t lock;
> > + void __iomem *base;
> > + uint32_t dir_in[3];
> > + int irq;
> > +};
> > +
> > +struct aspeed_sgpio_bank {
> > + uint16_tval_regs;
> > + uint16_trdata_reg;
> > + uint16_tirq_regs;
> > + const char names[4][3];
> > +};
> > +
> > +/*
> > + * Note: The "value" register returns the input value when the GPIO is
> > + * configured as an input.
> > + *
> > + * The "rdata" register returns the output value when the GPIO is
> > + * configured as an output.
> > + */
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > + {
> > + .val_regs = 0x,
> > + .rdata_reg = 0x0070,
> > + .irq_regs = 0x0004,
> > + .names = { "A", "B", "C", "D" },
> > + },
> > + {
> > + .val_regs = 0x001C,
> > + .rdata_reg = 0x0074,
> > + .irq_regs = 0x0020,
> > + .names = { "E", "F", &qu
dt-bindings: gpio: aspeed: Add SGPIO support
Hello Andrew,
Thanks for reviewing and please see my inline comments.
--Hongwei
> From: Andrew Jeffery
> Sent: Wednesday, July 17, 2019 9:48 PM
> To: Hongwei Zhang; Joel Stanley; Linus Walleij; devicet...@vger.kernel.org
> Cc: Rob Herring; Mark Rutland; Bartosz Golaszewski;
> linux-asp...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-g...@vger.kernel.org
> Subject: Re: [PATCH 2/3 v4] dt-bindings: gpio: aspeed: Add SGPIO support
>
> The subject is largely correct, but please see the discussion on the driver
> patch about how to clean up
> the [PATCH ...] prefix.
>
> On Thu, 18 Jul 2019, at 05:42, Hongwei Zhang wrote:
> > Add bindings to support SGPIO on AST2400 or AST2500.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > .../devicetree/bindings/gpio/sgpio-aspeed.txt | 55
> > ++
> > 1 file changed, 55 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > new file mode 100644
> > index 000..2d6305e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > @@ -0,0 +1,55 @@
> > +Aspeed SGPIO controller Device Tree Bindings
> > +---
> > +
> > +This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80
> > full
> > +featured Serial GPIOs. Each of the Serial GPIO pins can be programmed
> > to
> > +support the following options:
> > +- Support interrupt option for each input port and various interrupt
> > + sensitivity option (level-high, level-low, edge-high, edge-low)
> > +- Support reset tolerance option for each output port
> > +- Directly connected to APB bus and its shift clock is from APB bus
> > clock
> > + divided by a programmable value.
> > +- Co-work with external signal-chained TTL components
> > +(74LV165/74LV595)
> > +
> > +
> > +Required properties:
> > +
> > +- compatible : Either "aspeed,ast2400-sgpio" or
> > "aspeed,ast2500-sgpio"
> > +
> > +- #gpio-cells : Should be two
> > + - First cell is the GPIO line number
> > + - Second cell is used to specify optional
> > + parameters (unused)
> > +
> > +- reg : Address and length of the register set for
> > the device
> > +- gpio-controller : Marks the device node as a GPIO controller
> > +- interrupts : Interrupt specifier (see interrupt bindings
> > for
> > + details)
> > +
> > +- interrupt-controller : Mark the GPIO controller as an
> > interrupt-controller
> > +
> > +- nr-gpios : number of GPIO pins to serialise.
> > + (should be multiple of 8, up to 80 pins)
>
> Please change the property name to "ngpios", as per the generic GPIO
> bindings[1].
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindi
> ngs/gpio/gpio.txt?h=v5.2#n141
done
>
> Cheers,
>
> Andrew
>
> > +
> > +- clocks: A phandle to the APB clock for SGPM clock
> > division
> > +
> > +- bus-frequency: SGPM CLK frequency
> > +
> > +
> > +The sgpio and interrupt properties are further described in their
> > respective bindings documentation:
> > +
> > +- Documentation/devicetree/bindings/sgpio/gpio.txt
> > +-
> > +Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > +
> > + Example:
> > + sgpio: sgpio@1e780200 {
> > + #gpio-cells = <2>;
> > + compatible = "aspeed,ast2500-sgpio";
> > + gpio-controller;
> > + interrupts = <40>;
> > + reg = <0x1e780200 0x0100>;
> > + clocks = < ASPEED_CLK_APB>;
> > + interrupt-controller;
> > + nr-gpios = <8>;
> > + bus-frequency = <1200>;
> > + };
> > --
> > 2.7.4
> >
> >
[v5 2/2] gpio: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 522
1 file changed, 522 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..c024c0a
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,522 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem
[v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..f9ed438
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,55 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
+featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+support the following options:
+- Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+- Support reset tolerance option for each output port
+- Directly connected to APB bus and its shift clock is from APB bus clock
+ divided by a programmable value.
+- Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+
+Required properties:
+
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- ngpios : number of GPIO pins to serialise.
+ (should be multiple of 8, up to 80 pins)
+
+- clocks: A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ ngpios = <8>;
+ bus-frequency = <1200>;
+ };
--
2.7.4
[v5 0/2] gpio: aspeed: Add SGPIO driver
Hello, This short series introduce dt-binding document and a driver for the Aspeed AST2500 SGPIO controller. Please review. The related SGPM pinmux dt-binding document, dts, and pinctrl driver updates have been accepted and merged: _http://patchwork.ozlabs.org/patch/1110210/ Hongwei Zhang (2): dt-bindings: gpio: aspeed: Add SGPIO support gpio: aspeed: Add SGPIO driver .../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 +++ drivers/gpio/sgpio-aspeed.c| 522 + 2 files changed, 577 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt create mode 100644 drivers/gpio/sgpio-aspeed.c -- 2.7.4
[PATCH 2/3 v4] ARM: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 518
1 file changed, 518 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..715052c
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ uint32_t dir_in[3];
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value when the GPIO is
+ * configured as an input.
+ *
+ * The "rdata" register returns the output value when the GPIO is
+ * configured as an output.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ enum aspeed_sgpio_reg reg;
+ bool is_input;
+ int rc = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ is_input = gpio->dir_in[GPIO_BANK(offset)] & GPIO_BIT(offset);
+ reg = is_input ? reg_val : reg_rdata;
+ rc = !!(ioread32(bank_reg(gpio, bank, reg)) & GPIO_BIT(offset));
+
+ spin_unlock_irqrestore(>lock, flags);
+
+ return rc;
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem
RE: [PATCH 2/3 v3] dt-bindings: gpio: aspeed: Add SGPIO support
Hello Andrew,
Thanks for your review, please find the v4 in separate email. We merged all
your suggestion in v4.
Best Regards,
--Hongwei
-Original Message-
> From: Andrew Jeffery
> Sent: Tuesday, July 16, 2019 11:26 PM
> To: Hongwei Zhang; Bartosz Golaszewski; Joel Stanley; Linus Walleij
> Cc: linux-g...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-asp...@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3 v3] ARM: dts: aspeed: Add SGPIO driver
>
> Hello Hongwei,
>
> Please send patches and feedback on prior iterations separately. Please send
> the output of `git format-
> patch ...`directly; format-patch spits the patch out in email form ready to
> go and can be fed straight to
> `git send-email`.
>
> On Wed, 17 Jul 2019, at 06:54, Hongwei Zhang wrote:
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > drivers/gpio/sgpio-aspeed.c | 487
> >
> > 1 file changed, 487 insertions(+)
> > create mode 100644 drivers/gpio/sgpio-aspeed.c
> >
> > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > new file mode 100644 index 000..ade2cb7
> > --- /dev/null
> > +++ b/drivers/gpio/sgpio-aspeed.c
> > @@ -0,0 +1,487 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 American Megatrends International LLC.
> > + *
> > + * Author: Karthikeyan Mani */
> > +
> > +#include
> > +#include
>
> linux/gpio/aspeed.h is specific to the parallel GPIO driver, please drop this
> include.
>
Removed it.
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> This driver doesn't have any direct interaction with pinctrl, so I think we
> can remove this header
>
Removed it.
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define MAX_NR_SGPIO 80
> > +
> > +#define ASPEED_SGPIO_CTRL 0x54
> > +
> > +#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
> > +#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
> > +#define ASPEED_SGPIO_ENABLEBIT(0)
> > +
> > +// default sgpio direction is input.
> > +static uint32_t sgpio_dir_val[3] = {0x, 0x,
> > +0x
> > };
>
> Why not make it a member of struct aspeed_sgpio (below)? I'd prefer we encode
> the comment in the
> variable name as well, e.g.
> sgpio_dir_in`- this way when reading the code that uses it we know which bit
> state means what (set is
> input, clear is output).
>
Done.
> > +
> > +struct aspeed_sgpio {
> > + struct gpio_chip chip;
> > + struct clk *pclk;
> > + spinlock_t lock;
> > + void __iomem *base;
> > + int irq;
> > +};
> > +
> > +struct aspeed_sgpio_bank {
> > + uint16_tval_regs;
> > + uint16_trdata_reg;
> > + uint16_tirq_regs;
> > + const char names[4][3];
> > +};
> > +
> > +/*
> > + * Note: The "value" register returns the input value sampled on the
> > + * line even when the GPIO is configured as an output. Since
> > + * that input goes through synchronizers, writing, then reading
> > + * back may not return the written value right away.
>
> The paragraph above is somewhat specific to the parallel GPIO driver.
> It would be good to rework it for the context of the SGPIO driver.
> Documenting the split of the "value" and "rdata" register is a good thing.
>
> > + *
> > + * The "rdata" register returns the content of the write latch
> > + * and thus can be used to read back what was last written
> > + * reliably.
> > + */
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > + {
> > + .val_regs = 0x,
> > + .rdata_reg = 0x0070,
> > + .irq_regs = 0x0004,
> > + .names = { "A", "B", "C", "D" },
> > + },
> > + {
> > + .val_regs = 0x001C,
> > + .rdata_reg = 0x0074,
> > + .irq_regs = 0x0020,
> > + .names = { "E", "F", "G", "H" },
> > + },
> > + {
> > + .val_regs = 0x0038,
> > + .rdata_reg = 0x0078,
> > + .irq_regs = 0x
[PATCH 2/3 v4] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..2d6305e
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,55 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
+featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+support the following options:
+- Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+- Support reset tolerance option for each output port
+- Directly connected to APB bus and its shift clock is from APB bus clock
+ divided by a programmable value.
+- Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+
+Required properties:
+
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- nr-gpios : number of GPIO pins to serialise.
+ (should be multiple of 8, up to 80 pins)
+
+- clocks: A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio: sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ nr-gpios = <8>;
+ bus-frequency = <1200>;
+ };
--
2.7.4
[PATCH 2/3 v3] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 55 ++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..8c3a747
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,55 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+This SGPIO controller is for ASPEED AST2500 SoC, it supports up to 80 full
+featured Serial GPIOs. Each of the Serial GPIO pins can be programmed to
+support the following options:
+- Support interrupt option for each input port and various interrupt
+ sensitivity option (level-high, level-low, edge-high, edge-low)
+- Support reset tolerance option for each output port
+- Directly connected to APB bus and its shift clock is from APB bus clock
+ divided by a programmable value.
+- Co-work with external signal-chained TTL components (74LV165/74LV595)
+
+
+Required properties:
+
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- nr-gpios : number of GPIO pins to serialise.
+ (should be multiple of 8, up to 80 pins; 0 if not
used)
+
+- clocks : A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency, derived from APB bus clock by a
programmable devisor
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ nr-gpios = <8>;
+ bus-frequency = <1200>;
+ };
--
2.7.4
Thanks Andrew, please see above v3 and inline comments at below.
--Hongwei
> From: Andrew Jeffery
> Sent: Sunday, July 14, 2019 10:25 PM
> To: Hongwei Zhang; Joel Stanley; Linus Walleij; devicet...@vger.kernel.org
> Cc: Rob Herring; Mark Rutland; Bartosz Golaszewski;
> linux-asp...@lists.ozlabs.org; linux-
> ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-g...@vger.kernel.org
> Subject: Re: [PATCH 2/3 v2] dt-bindings: gpio: aspeed: Add SGPIO support
>
> Hello Hongwei,
>
> On Sat, 13 Jul 2019, at 05:44, Hongwei Zhang wrote:
> > Add bindings to support SGPIO on AST2400 or AST2500.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > .../devicetree/bindings/gpio/sgpio-aspeed.txt | 43
> > ++
> > 1 file changed, 43 insertions(+)
> > create mode 100755
> > Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > new file mode 100755
> > index 000..3ae2b79
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> > @@ -0,0 +1,43 @@
> > +Aspeed SGPIO controller Device Tree Bindings
> > +---
> > +
> > +Required properties:
> > +- compatible : Either "aspeed,ast2400-sgpio" or
> > "aspeed,ast2500-sgpio"
> > +
> > +- #gpio-cells : Should be two
> > + - First cell is the GPIO line number
> > + - Second cell is used to specify optional
> > + parameters (unused)
> > +
> > +- reg : Address and length of the register set for
> > the device
> > +- gpio-controller : Marks the device node as a GPIO controller.
> > +- interru
[PATCH 2/3 v3] ARM: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 487
1 file changed, 487 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..ade2cb7
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,487 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_NR_SGPIO 80
+
+#define ASPEED_SGPIO_CTRL 0x54
+
+#define ASPEED_SGPIO_PINS_MASK GENMASK(9, 6)
+#define ASPEED_SGPIO_CLK_DIV_MASK GENMASK(31, 16)
+#define ASPEED_SGPIO_ENABLEBIT(0)
+
+// default sgpio direction is input.
+static uint32_t sgpio_dir_val[3] = {0x, 0x, 0x };
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ struct clk *pclk;
+ spinlock_t lock;
+ void __iomem *base;
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value sampled on the
+ * line even when the GPIO is configured as an output. Since
+ * that input goes through synchronizers, writing, then reading
+ * back may not return the written value right away.
+ *
+ * The "rdata" register returns the content of the write latch
+ * and thus can be used to read back what was last written
+ * reliably.
+ */
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_VAL_DIR0x04
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ BUG_ON(1);
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+
+ if (sgpio_dir_val[GPIO_BANK(offset)] & GPIO_BIT(offset))
+ return !!(ioread32(bank_reg(gpio, bank, reg_val)) &
GPIO_BIT(offset));
+ else
+ return !!(ioread32(bank_reg(gpio, bank, reg_rdata)) &
GPIO_BIT(offset));
+
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset
RE: [linux,dev-5.1 v1] dt-bindings: gpio: aspeed: Add SGPIO support
Thanks for your review, Andrew,
Just submitted an updated binding document, with new proper subject line:
[PATCH 2/3 v2] dt-bindings: gpio: aspeed: Add SGPIO support
Regards,
--Hongwei
-Original Message-
From: Andrew Jeffery
Sent: Tuesday, July 9, 2019 10:16 PM
To: Hongwei Zhang ; devicet...@vger.kernel.org; Joel Stanley
; Linus Walleij
Cc: Rob Herring ; Mark Rutland ;
linux-asp...@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [linux,dev-5.1 v1] dt-bindings: gpio: aspeed: Add SGPIO support
On Thu, 4 Jul 2019, at 05:31, Hongwei Zhang wrote:
> Add bindings to support SGPIO on AST2400 or AST2500.
>
> Signed-off-by: Hongwei Zhang
> ---
> .../devicetree/bindings/gpio/sgpio-aspeed.txt | 36
> ++
> 1 file changed, 36 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> new file mode 100644
> index 000..f5fc6ef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
> @@ -0,0 +1,36 @@
> +Aspeed SGPIO controller Device Tree Bindings
> +---
> +
> +Required properties:
> +- compatible : Either "aspeed,ast2400-sgpio" or
> "aspeed,ast2500-sgpio"
> +
> +- #gpio-cells: Should be two
> + - First cell is the GPIO line number
> + - Second cell is used to specify optional
> + parameters (unused)
> +
> +- reg: Address and length of the register set for
> the device
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- interrupts : Interrupt specifier (see interrupt bindings for
> + details)
> +- interrupt-controller : Mark the GPIO controller as an
> interrupt-controller
As this is a serial GPIO controller, a critical piece of configuration
information is how many GPIOs we wish to serialise. This is done in multiples
of 8, up to 80 pins.
The bindings need to describe the "ngpios" property from the generic GPIO
bindings and how this affects the behaviour of the controller.
We also need to add the "bus-frequency" property here to control the rate of
SGPMCK.
> +
> +Optional properties:
> +
> +- clocks: A phandle to the clock to use for debounce
> timings
We need this, but not for the reason specified, and it should be a required
property. We need PCLK (the APB clock) to derive the SGPIO bus frequency.
Despite what the datasheet blurb says, there's no debounce control for the
SGPIO master (this is a copy/paste mistake from the description of the parallel
GPIO master).
> +
> +The sgpio and interrupt properties are further described in their
> respective
> +bindings documentation:
> +
> +- Documentation/devicetree/bindings/sgpio/gpio.txt
> +-
> +Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +
> + Example:
> + sgpio@1e780200 {
> + #gpio-cells = <2>;
> + compatible = "aspeed,ast2500-sgpio";
> + gpio-controller;
> + interrupts = <40>;
> + reg = <0x1e780200 0x0100>;
> + interrupt-controller;
> + };
You'll need to fix up the example after making the changes mentioned above.
Andrew
> --
> 2.7.4
>
>
[PATCH 2/3 v2] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 43 ++
1 file changed, 43 insertions(+)
create mode 100755 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100755
index 000..3ae2b79
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,43 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+- nr-gpios : number of GPIO pins to serialise. (should be multiple
of 8, up to 80 pins)
+ if not specified, defaults to 80.
+
+- clocks : A phandle to the APB clock for SGPM clock division
+
+- bus-frequency: SGPM CLK frequency, if not specified defaults to 1 MHz
+
+
+The sgpio and interrupt properties are further described in their respective
bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ clocks = < ASPEED_CLK_APB>;
+ interrupt-controller;
+ nr-gpios = <80>;
+ bus-frequency = <100>;
+ };
--
2.7.4
[PATCH 2/3 v2] ARM: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 450
1 file changed, 450 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..0743d22
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * Author: Karthikeyan Mani
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NR_SGPIO80
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ spinlock_t lock;
+ void __iomem *base;
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value sampled on the
+ * line even when the GPIO is configured as an output. Since
+ * that input goes through synchronizers, writing, then reading
+ * back may not return the written value right away.
+ *
+ * The "rdata" register returns the content of the write latch
+ * and thus can be used to read back what was last written
+ * reliably.
+ */
+
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_VAL_DIR0x04
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ default:
+ /* acturally if code runs to here, it's an error case */
+ WARN_ON(reg);
+ return gpio->base;
+ }
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+
+ return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
+}
+
+static void aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset, int
val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned long flags;
+ void __iomem *addr;
+ u32 reg = 0;
+
+ spin_lock_irqsave(>lock, flags);
+
+ addr = bank_reg(gpio, bank, reg_val);
+
+ if (val)
+ reg |= GPIO_BIT(offset);
+ else
+ reg &= ~GPIO_BIT(offset);
+
+ iowrite32(reg, addr);
+ spin_unlock_irqrestore(>lock, flags);
+}
+
+static int aspeed_sgpio_d
RE: [PATCH 2/3 v1] ARM: dts: aspeed: Add SGPIO driver
Hello Andrew,
Thanks for your review and comments, please find our inline response at below.
I will email updated driver code separately, because Outlook breaks source
code's tabs.
There is one place need your more input for clarification, which is about
DATA_READ/DATA_VALUE registers,
please see it at below.
Best Regards,
-- Hongwei
> From: Andrew Jeffery
> Sent: Wednesday, July 3, 2019 8:06 PM
> To: Hongwei Zhang; Bartosz Golaszewski; Joel Stanley; Linus Walleij
> Cc: linux-g...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-asp...@lists.ozlabs.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/3 linux,dev-5.1 v1] ARM: dts: aspeed: Add SGPIO
> driver
>
> Hello Hongwei,
>
> As this is patch is sent to the upstream lists (linux-gpio@ etc) please drop
> the OpenBMC-specific
> "linux,dev-5.1" from the subject.
>
Got it but to be more specific, for the situation of mixed recipients, should I
send out separate emails with
different subject line format in the future?
> Also, it looks like you may have manually added the series revision (v1).
> For the record you can make `git format-patch` do this for you with the
> `-v`option (e.g. if you really want
> it here, `-v 1`).
>
> On Thu, 4 Jul 2019, at 07:09, Hongwei Zhang wrote:
> > Add SGPIO driver support for Aspeed AST2500 SoC.
> >
> > Signed-off-by: Hongwei Zhang
> > ---
> > drivers/gpio/sgpio-aspeed.c | 470
> >
> > 1 file changed, 470 insertions(+)
> > create mode 100644 drivers/gpio/sgpio-aspeed.c
> >
> > diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
> > new file mode 100644 index 000..108ed13
> > --- /dev/null
> > +++ b/drivers/gpio/sgpio-aspeed.c
> > @@ -0,0 +1,470 @@
> > +/*
> > + * Copyright 2019 American Megatrends International LLC.
> > + *
> > + * 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 of the License, or (at your option) any later version.
>
> You should use the SPDX license identifier here rather than the GPL blurb,
> and it should be the first line
> of the file. Keep your copyright line in place though:
>
OK
> // SPDX-License-Identifier: GPL-2.0-or-later // Copyright 2019 American
> Megatrends International LLC.
>
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define NR_SGPIO80
> > +
> > +struct aspeed_sgpio {
> > + struct gpio_chip chip;
> > + spinlock_t lock;
> > + void __iomem *base;
> > + int irq;
> > +};
> > +
> > +struct aspeed_sgpio_bank {
> > + uint16_tval_regs;
> > + uint16_trdata_reg;
> > + uint16_tirq_regs;
> > + const char names[4][3];
> > +};
> > +
> > +/*
> > + * Note: The "value" register returns the input value sampled on the
> > + * line even when the GPIO is configured as an output. Since
> > + * that input goes through synchronizers, writing, then reading
> > + * back may not return the written value right away.
> > + *
> > + * The "rdata" register returns the content of the write latch
> > + * and thus can be used to read back what was last written
> > + * reliably.
> > + */
> > +
> > +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> > + {
> > + .val_regs = 0x,
> > + .rdata_reg = 0x0070,
> > + .irq_regs = 0x0004,
> > + .names = { "A", "B", "C", "D" },
> > + },
> > + {
> > + .val_regs = 0x001C,
> > + .rdata_reg = 0x0074,
> > + .irq_regs = 0x0020,
> > + .names = { "E", "F", "G", "H" },
> > + },
> > + {
> > + .val_regs = 0x0038,
> > + .rdata_reg = 0x0078,
> > + .irq_regs = 0x003C,
> > + .names = { "I", "J" },
> > + },
> > +};
> > +
> > +enum aspeed_sgpio_reg {
> > + reg_val,
> > + reg_rdata,
> > + reg_irq_enable,
> > + reg_irq_type0,
> > + reg_irq_type1,
> > + reg_irq_type2,
> > + reg_irq_
[PATCH 2/3 linux,dev-5.1 v1] ARM: dts: aspeed: Add SGPIO driver
Add SGPIO driver support for Aspeed AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
drivers/gpio/sgpio-aspeed.c | 470
1 file changed, 470 insertions(+)
create mode 100644 drivers/gpio/sgpio-aspeed.c
diff --git a/drivers/gpio/sgpio-aspeed.c b/drivers/gpio/sgpio-aspeed.c
new file mode 100644
index 000..108ed13
--- /dev/null
+++ b/drivers/gpio/sgpio-aspeed.c
@@ -0,0 +1,470 @@
+/*
+ * Copyright 2019 American Megatrends International LLC.
+ *
+ * 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 of the License, or (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define NR_SGPIO80
+
+struct aspeed_sgpio {
+ struct gpio_chip chip;
+ spinlock_t lock;
+ void __iomem *base;
+ int irq;
+};
+
+struct aspeed_sgpio_bank {
+ uint16_tval_regs;
+ uint16_trdata_reg;
+ uint16_tirq_regs;
+ const char names[4][3];
+};
+
+/*
+ * Note: The "value" register returns the input value sampled on the
+ * line even when the GPIO is configured as an output. Since
+ * that input goes through synchronizers, writing, then reading
+ * back may not return the written value right away.
+ *
+ * The "rdata" register returns the content of the write latch
+ * and thus can be used to read back what was last written
+ * reliably.
+ */
+
+static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
+ {
+ .val_regs = 0x,
+ .rdata_reg = 0x0070,
+ .irq_regs = 0x0004,
+ .names = { "A", "B", "C", "D" },
+ },
+ {
+ .val_regs = 0x001C,
+ .rdata_reg = 0x0074,
+ .irq_regs = 0x0020,
+ .names = { "E", "F", "G", "H" },
+ },
+ {
+ .val_regs = 0x0038,
+ .rdata_reg = 0x0078,
+ .irq_regs = 0x003C,
+ .names = { "I", "J" },
+ },
+};
+
+enum aspeed_sgpio_reg {
+ reg_val,
+ reg_rdata,
+ reg_irq_enable,
+ reg_irq_type0,
+ reg_irq_type1,
+ reg_irq_type2,
+ reg_irq_status,
+};
+
+#define GPIO_VAL_VALUE 0x00
+#define GPIO_VAL_DIR0x04
+#define GPIO_IRQ_ENABLE 0x00
+#define GPIO_IRQ_TYPE0 0x04
+#define GPIO_IRQ_TYPE1 0x08
+#define GPIO_IRQ_TYPE2 0x0C
+#define GPIO_IRQ_STATUS 0x10
+
+/* This will be resolved at compile time */
+static inline void __iomem *bank_reg(struct aspeed_sgpio *gpio,
+const struct aspeed_sgpio_bank *bank,
+const enum aspeed_sgpio_reg reg)
+{
+ switch (reg) {
+ case reg_val:
+ return gpio->base + bank->val_regs + GPIO_VAL_VALUE;
+ case reg_rdata:
+ return gpio->base + bank->rdata_reg;
+ case reg_irq_enable:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_ENABLE;
+ case reg_irq_type0:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE0;
+ case reg_irq_type1:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE1;
+ case reg_irq_type2:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_TYPE2;
+ case reg_irq_status:
+ return gpio->base + bank->irq_regs + GPIO_IRQ_STATUS;
+ }
+ BUG_ON(1);
+}
+
+#define GPIO_BANK(x)((x) >> 5)
+#define GPIO_OFFSET(x) ((x) & 0x1f)
+#define GPIO_BIT(x) BIT(GPIO_OFFSET(x))
+
+static const struct aspeed_sgpio_bank *to_bank(unsigned int offset)
+{
+ unsigned int bank = GPIO_BANK(offset);
+
+ WARN_ON(bank >= ARRAY_SIZE(aspeed_sgpio_banks));
+ return _sgpio_banks[bank];
+}
+
+static inline bool have_gpio(struct aspeed_sgpio *gpio, unsigned int offset)
+{
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ unsigned int group = GPIO_OFFSET(offset) / 8;
+
+ return bank->names[group][0] != '\0';
+}
+
+static int aspeed_sgpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+
+ return !!(ioread32(bank_reg(gpio, bank, reg_val)) & GPIO_BIT(offset));
+}
+
+static void __aspeed_sgpio_set(struct gpio_chip *gc, unsigned int offset,
+ int val)
+{
+ struct aspeed_sgpio *gpio = gpiochip_get_data(gc);
+ const struct aspeed_sgpio_bank *bank = to_bank(offset);
+ void __iomem *addr;
+ u32 reg;
+
+
[linux,dev-5.1 v1] dt-bindings: gpio: aspeed: Add SGPIO support
Add bindings to support SGPIO on AST2400 or AST2500.
Signed-off-by: Hongwei Zhang
---
.../devicetree/bindings/gpio/sgpio-aspeed.txt | 36 ++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
diff --git a/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
new file mode 100644
index 000..f5fc6ef
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/sgpio-aspeed.txt
@@ -0,0 +1,36 @@
+Aspeed SGPIO controller Device Tree Bindings
+---
+
+Required properties:
+- compatible : Either "aspeed,ast2400-sgpio" or
"aspeed,ast2500-sgpio"
+
+- #gpio-cells : Should be two
+ - First cell is the GPIO line number
+ - Second cell is used to specify optional
+ parameters (unused)
+
+- reg : Address and length of the register set for the device
+- gpio-controller : Marks the device node as a GPIO controller.
+- interrupts : Interrupt specifier (see interrupt bindings for
+ details)
+- interrupt-controller : Mark the GPIO controller as an interrupt-controller
+
+Optional properties:
+
+- clocks: A phandle to the clock to use for debounce timings
+
+The sgpio and interrupt properties are further described in their respective
+bindings documentation:
+
+- Documentation/devicetree/bindings/sgpio/gpio.txt
+- Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+ Example:
+ sgpio@1e780200 {
+ #gpio-cells = <2>;
+ compatible = "aspeed,ast2500-sgpio";
+ gpio-controller;
+ interrupts = <40>;
+ reg = <0x1e780200 0x0100>;
+ interrupt-controller;
+ };
--
2.7.4
RE: [PATCH linux dev-5.1 v1] ARM: dts: aspeed: Add SGPM pinmux
-Original Message- From: Linus Walleij Sent: Monday, June 3, 2019 7:08 PM To: Hongwei Zhang Cc: Joel Stanley ; Andrew Jeffery ; Rob Herring ; Mark Rutland ; open list:GPIO SUBSYSTEM ; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS ; Linux ARM ; linux-asp...@lists.ozlabs.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH linux dev-5.1 v1] ARM: dts: aspeed: Add SGPM pinmux Hi Hongwei, On Tue, Jun 4, 2019 at 12:44 AM Hongwei Zhang wrote: > > Add SGPM pinmux to ast2500-pinctrl function and group, to prepare for > supporting SGPIO in AST2500 SoC. > > Signed-off-by: Hongwei Zhang > --- > Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 2 +- > arch/arm/boot/dts/aspeed-g5.dtsi | 5 + > drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 4 Please try to separate out the change to arch/arm/boot/dts/aspeed-g5.dtsi into a separate patch that goes through ARM SoC. Just committed the DT patch to _https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git , please help to review. Thanks! Other than that it looks fine to me. Yours, Linus Walleij
[PATCH 1/3 linux dev-5.1 v2] ARM: dts: aspeed: Add SGPM pinmux
Add SGPM pinmux to ast2500-pinctrl function and group, to prepare for
supporting SGPIO in AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 2 +-
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
index 3b7266c..8f1c5c4 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
@@ -84,7 +84,7 @@ NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4 NDTR1 NDTR2 NDTR3
NDTR4 NRI1 NRI2
NRI3 NRI4 NRTS1 NRTS2 NRTS3 NRTS4 OSCCLK PEWAKE PNOR PWM0 PWM1 PWM2 PWM3 PWM4
PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 RXD1 RXD2 RXD3 RXD4 SALT1 SALT10
SALT11 SALT12 SALT13 SALT14 SALT2 SALT3 SALT4 SALT5 SALT6 SALT7 SALT8 SALT9
-SCL1 SCL2 SD1 SD2 SDA1 SDA2 SGPS1 SGPS2 SIOONCTRL SIOPBI SIOPBO SIOPWREQ
+SCL1 SCL2 SD1 SD2 SDA1 SDA2 SGPM SGPS1 SGPS2 SIOONCTRL SIOPBI SIOPBO SIOPWREQ
SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1CS1 SPI1DEBUG SPI1PASSTHRU SPI2CK SPI2CS0
SPI2CS1 SPI2MISO SPI2MOSI TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2
TXD3 TXD4 UART6 USB11BHID USB2AD USB2AH USB2BD USB2BH USBCKI VGABIOSROM VGAHS
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 187abd7..0c89647 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -577,6 +577,8 @@ SS_PIN_DECL(N3, GPIOJ2, SGPMO);
SIG_EXPR_LIST_DECL_SINGLE(SGPMI, SGPM, SIG_DESC_SET(SCU84, 11));
SS_PIN_DECL(N4, GPIOJ3, SGPMI);
+FUNC_GROUP_DECL(SGPM, R2, L2, N3, N4);
+
#define N5 76
SIG_EXPR_LIST_DECL_SINGLE(VGAHS, VGAHS, SIG_DESC_SET(SCU84, 12));
SIG_EXPR_LIST_DECL_SINGLE(DASHN5, DASHN5, SIG_DESC_SET(SCU94, 8));
@@ -2127,6 +2129,7 @@ static const struct aspeed_pin_group aspeed_g5_groups[] =
{
ASPEED_PINCTRL_GROUP(SD2),
ASPEED_PINCTRL_GROUP(SDA1),
ASPEED_PINCTRL_GROUP(SDA2),
+ ASPEED_PINCTRL_GROUP(SGPM),
ASPEED_PINCTRL_GROUP(SGPS1),
ASPEED_PINCTRL_GROUP(SGPS2),
ASPEED_PINCTRL_GROUP(SIOONCTRL),
@@ -2296,6 +2299,7 @@ static const struct aspeed_pin_function
aspeed_g5_functions[] = {
ASPEED_PINCTRL_FUNC(SD2),
ASPEED_PINCTRL_FUNC(SDA1),
ASPEED_PINCTRL_FUNC(SDA2),
+ ASPEED_PINCTRL_FUNC(SGPM),
ASPEED_PINCTRL_FUNC(SGPS1),
ASPEED_PINCTRL_FUNC(SGPS2),
ASPEED_PINCTRL_FUNC(SIOONCTRL),
--
2.7.4
[PATCH 1/3 linux dev-5.1 arm/soc v2] ARM: dts: aspeed: Add SGPM pinmux
Add SGPM pinmux to ast2500-pinctrl function and group, to prepare for
supporting SGPIO in AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
arch/arm/boot/dts/aspeed-g5.dtsi | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 85ed9db..8d30818 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -1321,6 +1321,11 @@
groups = "SDA2";
};
+ pinctrl_sgpm_default: sgpm_default {
+ function = "SGPM";
+ groups = "SGPM";
+ };
+
pinctrl_sgps1_default: sgps1_default {
function = "SGPS1";
groups = "SGPS1";
--
2.7.4
[PATCH linux dev-5.1 v1] ARM: dts: aspeed: Add SGPM pinmux
Add SGPM pinmux to ast2500-pinctrl function and group, to prepare for
supporting SGPIO in AST2500 SoC.
Signed-off-by: Hongwei Zhang
---
Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 2 +-
arch/arm/boot/dts/aspeed-g5.dtsi | 5 +
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 4
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
index 3b7266c..8f1c5c4 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
@@ -84,7 +84,7 @@ NDCD2 NDCD3 NDCD4 NDSR1 NDSR2 NDSR3 NDSR4 NDTR1 NDTR2 NDTR3
NDTR4 NRI1 NRI2
NRI3 NRI4 NRTS1 NRTS2 NRTS3 NRTS4 OSCCLK PEWAKE PNOR PWM0 PWM1 PWM2 PWM3 PWM4
PWM5 PWM6 PWM7 RGMII1 RGMII2 RMII1 RMII2 RXD1 RXD2 RXD3 RXD4 SALT1 SALT10
SALT11 SALT12 SALT13 SALT14 SALT2 SALT3 SALT4 SALT5 SALT6 SALT7 SALT8 SALT9
-SCL1 SCL2 SD1 SD2 SDA1 SDA2 SGPS1 SGPS2 SIOONCTRL SIOPBI SIOPBO SIOPWREQ
+SCL1 SCL2 SD1 SD2 SDA1 SDA2 SGPM SGPS1 SGPS2 SIOONCTRL SIOPBI SIOPBO SIOPWREQ
SIOPWRGD SIOS3 SIOS5 SIOSCI SPI1 SPI1CS1 SPI1DEBUG SPI1PASSTHRU SPI2CK SPI2CS0
SPI2CS1 SPI2MISO SPI2MOSI TIMER3 TIMER4 TIMER5 TIMER6 TIMER7 TIMER8 TXD1 TXD2
TXD3 TXD4 UART6 USB11BHID USB2AD USB2AH USB2BD USB2BH USBCKI VGABIOSROM VGAHS
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 383510d..d10c3ea 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -1394,6 +1394,11 @@
groups = "SDA2";
};
+ pinctrl_sgpm_default: sgpm_default {
+ function = "SGPM";
+ groups = "SGPM";
+ };
+
pinctrl_sgps1_default: sgps1_default {
function = "SGPS1";
groups = "SGPS1";
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 187abd7..0c89647 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -577,6 +577,8 @@ SS_PIN_DECL(N3, GPIOJ2, SGPMO);
SIG_EXPR_LIST_DECL_SINGLE(SGPMI, SGPM, SIG_DESC_SET(SCU84, 11));
SS_PIN_DECL(N4, GPIOJ3, SGPMI);
+FUNC_GROUP_DECL(SGPM, R2, L2, N3, N4);
+
#define N5 76
SIG_EXPR_LIST_DECL_SINGLE(VGAHS, VGAHS, SIG_DESC_SET(SCU84, 12));
SIG_EXPR_LIST_DECL_SINGLE(DASHN5, DASHN5, SIG_DESC_SET(SCU94, 8));
@@ -2127,6 +2129,7 @@ static const struct aspeed_pin_group aspeed_g5_groups[] =
{
ASPEED_PINCTRL_GROUP(SD2),
ASPEED_PINCTRL_GROUP(SDA1),
ASPEED_PINCTRL_GROUP(SDA2),
+ ASPEED_PINCTRL_GROUP(SGPM),
ASPEED_PINCTRL_GROUP(SGPS1),
ASPEED_PINCTRL_GROUP(SGPS2),
ASPEED_PINCTRL_GROUP(SIOONCTRL),
@@ -2296,6 +2299,7 @@ static const struct aspeed_pin_function
aspeed_g5_functions[] = {
ASPEED_PINCTRL_FUNC(SD2),
ASPEED_PINCTRL_FUNC(SDA1),
ASPEED_PINCTRL_FUNC(SDA2),
+ ASPEED_PINCTRL_FUNC(SGPM),
ASPEED_PINCTRL_FUNC(SGPS1),
ASPEED_PINCTRL_FUNC(SGPS2),
ASPEED_PINCTRL_FUNC(SIOONCTRL),
--
2.7.4
