Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1709680 > unrolled thread
| Started by | Yong Li <sdliyong@gmail.com> |
|---|---|
| First post | 2017-08-11 16:30 +0200 |
| Last post | 2017-08-14 03:00 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic Yong Li <sdliyong@gmail.com> - 2017-08-11 16:30 +0200
Re: [PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic Andrew Jeffery <andrew@aj.id.au> - 2017-08-14 03:00 +0200
| From | Yong Li <sdliyong@gmail.com> |
|---|---|
| Date | 2017-08-11 16:30 +0200 |
| Subject | [PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic |
| Message-ID | <udj6P-4Eb-35@gated-at.bofh.it> |
On AST2500, the hardware strap register(SCU70) only accepts write ‘1’,
to clear it to ‘0’, must set bits(write ‘1’) to SCU7C
Signed-off-by: Yong Li <sdliyong@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 20 ++++++++++++++++++--
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index a86a4d6..9d2b2e9 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -183,6 +183,7 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
{
int ret;
int i;
+ unsigned int rev_id;
for (i = 0; i < expr->ndescs; i++) {
const struct aspeed_sig_desc *desc = &expr->descs[i];
@@ -213,8 +214,23 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
continue;
- ret = regmap_update_bits(maps[desc->ip], desc->reg,
- desc->mask, val);
+ /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+ val == 0) {
+ ret = regmap_read(maps[ASPEED_IP_SCU],
+ HW_REVISION_ID, &rev_id);
+ if (ret < 0)
+ return ret;
+
+ if (0x04 == ((rev_id >> 24) & 0xff))
+ ret = regmap_update_bits(maps[desc->ip],
+ HW_REVISION_ID, desc->mask, desc->mask);
+ else
+ ret = regmap_update_bits(maps[desc->ip],
+ desc->reg, desc->mask, val);
+ } else
+ ret = regmap_update_bits(maps[desc->ip], desc->reg,
+ desc->mask, val);
if (ret)
return ret;
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index fa125db..d4d7f03 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -251,6 +251,7 @@
#define SCU3C 0x3C /* System Reset Control/Status Register */
#define SCU48 0x48 /* MAC Interface Clock Delay Setting */
#define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
+#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
#define SCU80 0x80 /* Multi-function Pin Control #1 */
#define SCU84 0x84 /* Multi-function Pin Control #2 */
#define SCU88 0x88 /* Multi-function Pin Control #3 */
--
2.7.4
[toc] | [next] | [standalone]
| From | Andrew Jeffery <andrew@aj.id.au> |
|---|---|
| Date | 2017-08-14 03:00 +0200 |
| Subject | Re: [PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic |
| Message-ID | <uebTz-5I6-1@gated-at.bofh.it> |
| In reply to | #1709680 |
[Multipart message — attachments visible in raw view] — view raw
Hi Yong,
On Fri, 2017-08-11 at 22:22 +0800, Yong Li wrote:
> On AST2500, the hardware strap register(SCU70) only accepts write ‘1’,
> to clear it to ‘0’, must set bits(write ‘1’) to SCU7C
>
> > Signed-off-by: Yong Li <sdliyong@gmail.com>
> ---
> drivers/pinctrl/aspeed/pinctrl-aspeed.c | 20 ++++++++++++++++++--
> drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> index a86a4d6..9d2b2e9 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
> @@ -183,6 +183,7 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> {
> > int ret;
> > int i;
> > + unsigned int rev_id;
>
> > for (i = 0; i < expr->ndescs; i++) {
> > const struct aspeed_sig_desc *desc = &expr->descs[i];
> @@ -213,8 +214,23 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
> > if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
> > continue;
>
> > - ret = regmap_update_bits(maps[desc->ip], desc->reg,
> > - desc->mask, val);
> > + /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
> > + if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
> + val == 0) {
The AST2500 strapping register contains several multi-bit bit-fields.
Currently we know we'll only reach this test if we're modifying the
GPIO passthrough bits for bank D and E (bits 21 and 22 respectively),
so the 'val == 0' test is functional but I wonder if we shouldn't be
more flexible. If we're more flexible here then we may only need to
modify the strap register test above this hunk to change the behaviour,
rather than needing to rework your additions here as well. More on this
below.
> + ret = regmap_read(maps[ASPEED_IP_SCU],
> > + HW_REVISION_ID, &rev_id);
> > + if (ret < 0)
> > + return ret;
> +
> > + if (0x04 == ((rev_id >> 24) & 0xff))
> > + ret = regmap_update_bits(maps[desc->ip],
> + HW_REVISION_ID, desc->mask, desc->mask);
regmap_update_bits() will do a read-modify-write operation whilst only
taking the regmap lock once, making the modification atomic. However,
HW_REVISION_ID is a W1C (write 1 to clear) register associated with
SCU70, so we don't actually need the read operation under the lock. We
can simply use regmap_write() here.
Addressing my comment on 'val == 0' above, I think we can remove that
test if we instead write this as:
ret = regmap_write(maps[ desc->ip], HW_REVISION_ID, (~val & desc->mask));
That way if we ever expand the HW_STRAP1 capabilites to cover the
bitfields (e.g. maybe we want to be able to dynamically switch SPI into
debug mode) the code should (hopefully) just work.
Cheers,
Andrew
> + else
> > + ret = regmap_update_bits(maps[desc->ip],
> > + desc->reg, desc->mask, val);
> > + } else
> + ret = regmap_update_bits(maps[desc->ip], desc->reg,
> > + desc->mask, val);
>
> > if (ret)
> > return ret;
> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> index fa125db..d4d7f03 100644
> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
> @@ -251,6 +251,7 @@
> #define SCU3C 0x3C /* System Reset Control/Status Register */
> #define SCU48 0x48 /* MAC Interface Clock Delay Setting */
> #define HW_STRAP1 0x70 /* AST2400 strapping is 33 bits, is split */
> +#define HW_REVISION_ID 0x7C /* Silicon revision ID register */
> #define SCU80 0x80 /* Multi-function Pin Control #1 */
> #define SCU84 0x84 /* Multi-function Pin Control #2 */
> #define SCU88 0x88 /* Multi-function Pin Control #3 */
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web