From: Steven Lee <steven_lee@aspeedtech.com> To: Andrew Jeffery <andrew@aj.id.au> Cc: Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Rob Herring <robh+dt@kernel.org>, Joel Stanley <joel@jms.id.au>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "moderated list:ARM/ASPEED MACHINE SUPPORT" <linux-arm-kernel@lists.infradead.org>, "moderated list:ARM/ASPEED MACHINE SUPPORT" <linux-aspeed@lists.ozlabs.org>, open list <linux-kernel@vger.kernel.org>, Hongwei Zhang <Hongweiz@ami.com>, Ryan Chen <ryan_chen@aspeedtech.com>, Billy Tsai <billy_tsai@aspeedtech.com> Subject: Re: [PATCH v5 06/10] gpio: gpio-aspeed-sgpio: Add AST2400 and AST2500 platform data. Date: Wed, 9 Jun 2021 12:12:28 +0800 [thread overview] Message-ID: <20210609041227.GB14839@aspeedtech.com> (raw) In-Reply-To: <6f87ccf4-9b8f-4c67-84a1-e83a2ee5103b@www.fastmail.com> The 06/09/2021 08:55, Andrew Jeffery wrote: > > > On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote: > > We use platform data to store GPIO pin mask and the max number of > > available GPIO pins for AST2600. > > Refactor driver to also add the platform data for AST2400/AST2500 and > > remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros. > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > --- > > drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++--------------------- > > 1 file changed, 12 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c > > index ea20a0127748..7d0a4f6fd9d1 100644 > > --- a/drivers/gpio/gpio-aspeed-sgpio.c > > +++ b/drivers/gpio/gpio-aspeed-sgpio.c > > @@ -17,21 +17,8 @@ > > #include <linux/spinlock.h> > > #include <linux/string.h> > > > > -/* > > - * MAX_NR_HW_GPIO represents the number of actual hardware-supported GPIOs (ie, > > - * slots within the clocked serial GPIO data). Since each HW GPIO is both an > > - * input and an output, we provide MAX_NR_HW_GPIO * 2 lines on our gpiochip > > - * device. > > - * > > - * We use SGPIO_OUTPUT_OFFSET to define the split between the inputs and > > - * outputs; the inputs start at line 0, the outputs start at OUTPUT_OFFSET. > > - */ > > -#define MAX_NR_HW_SGPIO 80 > > -#define SGPIO_OUTPUT_OFFSET MAX_NR_HW_SGPIO > > - > > #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_ENABLE BIT(0) > > #define ASPEED_SGPIO_PINS_SHIFT 6 > > @@ -484,6 +471,11 @@ static int aspeed_sgpio_setup_irqs(struct > > aspeed_sgpio *gpio, > > return 0; > > } > > > > +static const struct aspeed_sgpio_pdata ast2400_sgpio_pdata = { > > + .max_ngpios = 80, > > + .pin_mask = GENMASK(9, 6), > > +}; > > + > > static const struct aspeed_sgpio_pdata ast2600_sgpiom_128_pdata = { > > .max_ngpios = 128, > > .pin_mask = GENMASK(10, 6), > > @@ -495,8 +487,8 @@ static const struct aspeed_sgpio_pdata > > ast2600_sgpiom_80_pdata = { > > }; > > > > static const struct of_device_id aspeed_sgpio_of_table[] = { > > - { .compatible = "aspeed,ast2400-sgpio" }, > > - { .compatible = "aspeed,ast2500-sgpio" }, > > + { .compatible = "aspeed,ast2400-sgpio", .data = &ast2400_sgpio_pdata, > > }, > > + { .compatible = "aspeed,ast2500-sgpio", .data = &ast2400_sgpio_pdata, > > }, > > { .compatible = "aspeed,ast2600-sgpiom-128", .data = > > &ast2600_sgpiom_128_pdata, }, > > { .compatible = "aspeed,ast2600-sgpiom-80", .data = > > &ast2600_sgpiom_80_pdata, }, > > {} > > @@ -521,13 +513,11 @@ static int __init aspeed_sgpio_probe(struct > > platform_device *pdev) > > return PTR_ERR(gpio->base); > > > > pdata = device_get_match_data(&pdev->dev); > > - if (pdata) { > > - gpio->max_ngpios = pdata->max_ngpios; > > - pin_mask = pdata->pin_mask; > > - } else { > > - gpio->max_ngpios = MAX_NR_HW_SGPIO; > > - pin_mask = ASPEED_SGPIO_PINS_MASK; > > - } > > + if (!pdata) > > + return -EINVAL; > > + > > + gpio->max_ngpios = pdata->max_ngpios; > > + pin_mask = pdata->pin_mask; > > Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch. > > I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered. > I was wondering if I can squash patch-05 and patch-06 into one patch as this patch(patch-06) requires macros, structures, and functions that modified in the previous patch(patch-05). Thanks, Steven > Andrew
WARNING: multiple messages have this Message-ID (diff)
From: Steven Lee <steven_lee@aspeedtech.com> To: Andrew Jeffery <andrew@aj.id.au> Cc: Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Rob Herring <robh+dt@kernel.org>, Joel Stanley <joel@jms.id.au>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "moderated list:ARM/ASPEED MACHINE SUPPORT" <linux-arm-kernel@lists.infradead.org>, "moderated list:ARM/ASPEED MACHINE SUPPORT" <linux-aspeed@lists.ozlabs.org>, open list <linux-kernel@vger.kernel.org>, Hongwei Zhang <Hongweiz@ami.com>, Ryan Chen <ryan_chen@aspeedtech.com>, Billy Tsai <billy_tsai@aspeedtech.com> Subject: Re: [PATCH v5 06/10] gpio: gpio-aspeed-sgpio: Add AST2400 and AST2500 platform data. Date: Wed, 9 Jun 2021 12:12:28 +0800 [thread overview] Message-ID: <20210609041227.GB14839@aspeedtech.com> (raw) In-Reply-To: <6f87ccf4-9b8f-4c67-84a1-e83a2ee5103b@www.fastmail.com> The 06/09/2021 08:55, Andrew Jeffery wrote: > > > On Tue, 8 Jun 2021, at 19:55, Steven Lee wrote: > > We use platform data to store GPIO pin mask and the max number of > > available GPIO pins for AST2600. > > Refactor driver to also add the platform data for AST2400/AST2500 and > > remove unused MAX_NR_HW_SGPIO and ASPEED_SGPIO_PINS_MASK macros. > > > > Signed-off-by: Steven Lee <steven_lee@aspeedtech.com> > > --- > > drivers/gpio/gpio-aspeed-sgpio.c | 34 +++++++++++--------------------- > > 1 file changed, 12 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpio/gpio-aspeed-sgpio.c b/drivers/gpio/gpio-aspeed-sgpio.c > > index ea20a0127748..7d0a4f6fd9d1 100644 > > --- a/drivers/gpio/gpio-aspeed-sgpio.c > > +++ b/drivers/gpio/gpio-aspeed-sgpio.c > > @@ -17,21 +17,8 @@ > > #include <linux/spinlock.h> > > #include <linux/string.h> > > > > -/* > > - * MAX_NR_HW_GPIO represents the number of actual hardware-supported GPIOs (ie, > > - * slots within the clocked serial GPIO data). Since each HW GPIO is both an > > - * input and an output, we provide MAX_NR_HW_GPIO * 2 lines on our gpiochip > > - * device. > > - * > > - * We use SGPIO_OUTPUT_OFFSET to define the split between the inputs and > > - * outputs; the inputs start at line 0, the outputs start at OUTPUT_OFFSET. > > - */ > > -#define MAX_NR_HW_SGPIO 80 > > -#define SGPIO_OUTPUT_OFFSET MAX_NR_HW_SGPIO > > - > > #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_ENABLE BIT(0) > > #define ASPEED_SGPIO_PINS_SHIFT 6 > > @@ -484,6 +471,11 @@ static int aspeed_sgpio_setup_irqs(struct > > aspeed_sgpio *gpio, > > return 0; > > } > > > > +static const struct aspeed_sgpio_pdata ast2400_sgpio_pdata = { > > + .max_ngpios = 80, > > + .pin_mask = GENMASK(9, 6), > > +}; > > + > > static const struct aspeed_sgpio_pdata ast2600_sgpiom_128_pdata = { > > .max_ngpios = 128, > > .pin_mask = GENMASK(10, 6), > > @@ -495,8 +487,8 @@ static const struct aspeed_sgpio_pdata > > ast2600_sgpiom_80_pdata = { > > }; > > > > static const struct of_device_id aspeed_sgpio_of_table[] = { > > - { .compatible = "aspeed,ast2400-sgpio" }, > > - { .compatible = "aspeed,ast2500-sgpio" }, > > + { .compatible = "aspeed,ast2400-sgpio", .data = &ast2400_sgpio_pdata, > > }, > > + { .compatible = "aspeed,ast2500-sgpio", .data = &ast2400_sgpio_pdata, > > }, > > { .compatible = "aspeed,ast2600-sgpiom-128", .data = > > &ast2600_sgpiom_128_pdata, }, > > { .compatible = "aspeed,ast2600-sgpiom-80", .data = > > &ast2600_sgpiom_80_pdata, }, > > {} > > @@ -521,13 +513,11 @@ static int __init aspeed_sgpio_probe(struct > > platform_device *pdev) > > return PTR_ERR(gpio->base); > > > > pdata = device_get_match_data(&pdev->dev); > > - if (pdata) { > > - gpio->max_ngpios = pdata->max_ngpios; > > - pin_mask = pdata->pin_mask; > > - } else { > > - gpio->max_ngpios = MAX_NR_HW_SGPIO; > > - pin_mask = ASPEED_SGPIO_PINS_MASK; > > - } > > + if (!pdata) > > + return -EINVAL; > > + > > + gpio->max_ngpios = pdata->max_ngpios; > > + pin_mask = pdata->pin_mask; > > Hmm, okay, maybe just re-order the patches so this commit comes before the previous one. That way we don't immediately rip out this condition that we just introduced in the previous patch. > > I think I suggested squashing it into the previous patch, but with the removal of the comments and macros I think it's worth leaving it separate, just reordered. > I was wondering if I can squash patch-05 and patch-06 into one patch as this patch(patch-06) requires macros, structures, and functions that modified in the previous patch(patch-05). Thanks, Steven > Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-09 4:13 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-08 10:25 [PATCH v5 00/10] ASPEED sgpio driver enhancement Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-08 10:25 ` [PATCH v5 01/10] dt-bindings: aspeed-sgpio: Convert txt bindings to yaml Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:42 ` Andrew Jeffery 2021-06-09 0:42 ` Andrew Jeffery 2021-06-10 16:18 ` Rob Herring 2021-06-10 16:18 ` Rob Herring 2021-06-08 10:25 ` [PATCH v5 02/10] dt-bindings: aspeed-sgpio: Add ast2600 sgpio compatibles Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-10 16:23 ` Rob Herring 2021-06-10 16:23 ` Rob Herring 2021-06-10 23:27 ` Andrew Jeffery 2021-06-10 23:27 ` Andrew Jeffery 2021-06-16 15:20 ` Rob Herring 2021-06-16 15:20 ` Rob Herring 2021-06-08 10:25 ` [PATCH v5 03/10] ARM: dts: aspeed-g6: Add SGPIO node Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:43 ` Andrew Jeffery 2021-06-09 0:43 ` Andrew Jeffery 2021-06-09 1:51 ` Steven Lee 2021-06-09 1:51 ` Steven Lee 2021-06-09 2:19 ` Andrew Jeffery 2021-06-09 2:19 ` Andrew Jeffery 2021-06-08 10:25 ` [PATCH v5 04/10] ARM: dts: aspeed-g5: Remove ngpios from sgpio node Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:45 ` Andrew Jeffery 2021-06-09 0:45 ` Andrew Jeffery 2021-06-08 10:25 ` [PATCH v5 05/10] gpio: gpio-aspeed-sgpio: Add AST2600 sgpio support Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:52 ` Andrew Jeffery 2021-06-09 0:52 ` Andrew Jeffery 2021-06-08 10:25 ` [PATCH v5 06/10] gpio: gpio-aspeed-sgpio: Add AST2400 and AST2500 platform data Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:55 ` Andrew Jeffery 2021-06-09 0:55 ` Andrew Jeffery 2021-06-09 4:12 ` Steven Lee [this message] 2021-06-09 4:12 ` Steven Lee 2021-06-09 6:45 ` Andrew Jeffery 2021-06-09 6:45 ` Andrew Jeffery 2021-06-11 19:02 ` Bartosz Golaszewski 2021-06-11 19:02 ` Bartosz Golaszewski 2021-06-15 4:22 ` Steven Lee 2021-06-15 4:22 ` Steven Lee 2021-06-08 10:25 ` [PATCH v5 07/10] gpio: gpio-aspeed-sgpio: Add set_config function Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-08 10:25 ` [PATCH v5 08/10] gpio: gpio-aspeed-sgpio: Move irq_chip to aspeed-sgpio struct Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-08 10:25 ` [PATCH v5 09/10] gpio: gpio-aspeed-sgpio: Use generic device property APIs Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-08 10:25 ` [PATCH v5 10/10] gpio: gpio-aspeed-sgpio: Return error if ngpios is not multiple of 8 Steven Lee 2021-06-08 10:25 ` Steven Lee 2021-06-09 0:56 ` Andrew Jeffery 2021-06-09 0:56 ` Andrew Jeffery 2021-06-09 10:54 ` [PATCH v5 00/10] ASPEED sgpio driver enhancement Linus Walleij 2021-06-09 10:54 ` Linus Walleij 2021-06-10 2:24 ` Steven Lee 2021-06-10 2:24 ` Steven Lee 2021-06-10 7:50 ` Linus Walleij 2021-06-10 7:50 ` Linus Walleij 2021-06-10 8:39 ` Steven Lee 2021-06-10 8:39 ` Steven Lee
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210609041227.GB14839@aspeedtech.com \ --to=steven_lee@aspeedtech.com \ --cc=Hongweiz@ami.com \ --cc=andrew@aj.id.au \ --cc=bgolaszewski@baylibre.com \ --cc=billy_tsai@aspeedtech.com \ --cc=devicetree@vger.kernel.org \ --cc=joel@jms.id.au \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-aspeed@lists.ozlabs.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=ryan_chen@aspeedtech.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.