[PATCH v4] pinctrl: aspeed: Fix ast2500 strap register write logic
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 <sdliy...@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 21 +
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index a86a4d6..7f13ce8 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -213,6 +213,27 @@ static int aspeed_sig_expr_set(const struct
aspeed_sig_expr *expr,
if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
continue;
+ /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
+ unsigned int rev_id;
+
+ ret = regmap_read(maps[ASPEED_IP_SCU],
+ HW_REVISION_ID, _id);
+ if (ret < 0)
+ return ret;
+
+ if (0x04 == (rev_id >> 24)) {
+ u32 value = ~val & desc->mask;
+
+ if (value) {
+ ret = regmap_write(maps[desc->ip],
+ HW_REVISION_ID, value);
+ if (ret < 0)
+ return ret;
+ }
+ }
+ }
+
ret = regmap_update_bits(maps[desc->ip], desc->reg,
desc->mask, val);
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
[PATCH v4] pinctrl: aspeed: Fix ast2500 strap register write logic
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
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 21 +
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index a86a4d6..7f13ce8 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -213,6 +213,27 @@ static int aspeed_sig_expr_set(const struct
aspeed_sig_expr *expr,
if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
continue;
+ /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
+ if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
+ unsigned int rev_id;
+
+ ret = regmap_read(maps[ASPEED_IP_SCU],
+ HW_REVISION_ID, _id);
+ if (ret < 0)
+ return ret;
+
+ if (0x04 == (rev_id >> 24)) {
+ u32 value = ~val & desc->mask;
+
+ if (value) {
+ ret = regmap_write(maps[desc->ip],
+ HW_REVISION_ID, value);
+ if (ret < 0)
+ return ret;
+ }
+ }
+ }
+
ret = regmap_update_bits(maps[desc->ip], desc->reg,
desc->mask, val);
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
Re: [PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
Thanks Andrew for your thoughtful input. Let me send out another version :)
2017-08-16 20:19 GMT-07:00 Andrew Jeffery <and...@aj.id.au>:
> Hi Yong,
>
> On Wed, 2017-08-16 at 08:05 -0700, Yong Li wrote:
>> Hi Andrew,
>>
>> Thanks for your review. I checked the patch before I sent out, but the
>> tool did not report any problems. Could you help to share your
>> checking commands?
>>
>> scripts/checkpatch.pl
>> 0001-pinctrl-aspeed-Fix-ast2500-strap-register-write-logi.patch
>> total: 0 errors, 0 warnings, 38 lines checked
>
> Gah, turns out it was a problem with my mail client, which violates RFC4155
> and
> writes an mbox with CRLF. Awesome.
>
> I grabbed the mbox from patchwork and it was fine. Sorry for the noise.
>
> Separately, I slept on this and think there's a remaining issue. I'll outline
> it below in context.
>
>>
>> Thanks,
>> Yong
>>
>> > 2017-08-16 6:45 GMT-07:00 Andrew Jeffery <and...@aj.id.au>:
>> > Hi Yong,
>> >
>> > On Wed, 2017-08-16 at 00:21 +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 <sdliy...@gmail.com>
>> >
>> > ./scripts/checkpatch.pl complains about DOS line-endings thoughout -
>> > you should fix your editor to use Unix line endings (at least for
>> > kernel work). Maybe if Linus is feeling charitable he can fix it up for
>> > you. Regarding the meat of the change:
>> >
>> > > > Reviewed-by: Andrew Jeffery <and...@aj.id.au>
>> > > > Tested-by: Andrew Jeffery <and...@aj.id.au>
>> >
>> > Thanks for the patch!
>> >
>> > Andrew
>> >
>> > > ---
>> > > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
>> > > drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
>> > > 2 files changed, 18 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> > > b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> > > index a86a4d6..f2d5133 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 = >descs[i];
>> > >
>> > > @@ -213,8 +214,22 @@ 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) {
>> > > > + ret = regmap_read(maps[ASPEED_IP_SCU],
>> > > > + HW_REVISION_ID, _id);
>> > > > + if (ret < 0)
>> > > > + return ret;
>> > >
>> > > +
>> > > > + if (0x04 == ((rev_id >> 24) & 0xff))
>> > > > + ret = regmap_write(maps[desc->ip],
>> > > > + HW_REVISION_ID, (~val &
>> > > > desc->mask));
>> > > > + else
>
> Looking back at v2 what you had was correct for the single-bit field case (if
> 'val == 0' was false we went and wrote the set bit to HW_STRAP1), and it seems
> I didn't think through my suggestion enough to catch the problem I created.
>
> Essentially we should drop the 'else' above and always perform the
> regmap_update_bits() HW_STRAP1 because it is write-1-set. Otherwise we can
> only clear bits in the strapping register on the AST2500 and not set them.
> However, given the duplication pointed out below, I've suggested a further
> rearrangement.
>
>> &
Re: [PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
Thanks Andrew for your thoughtful input. Let me send out another version :)
2017-08-16 20:19 GMT-07:00 Andrew Jeffery :
> Hi Yong,
>
> On Wed, 2017-08-16 at 08:05 -0700, Yong Li wrote:
>> Hi Andrew,
>>
>> Thanks for your review. I checked the patch before I sent out, but the
>> tool did not report any problems. Could you help to share your
>> checking commands?
>>
>> scripts/checkpatch.pl
>> 0001-pinctrl-aspeed-Fix-ast2500-strap-register-write-logi.patch
>> total: 0 errors, 0 warnings, 38 lines checked
>
> Gah, turns out it was a problem with my mail client, which violates RFC4155
> and
> writes an mbox with CRLF. Awesome.
>
> I grabbed the mbox from patchwork and it was fine. Sorry for the noise.
>
> Separately, I slept on this and think there's a remaining issue. I'll outline
> it below in context.
>
>>
>> Thanks,
>> Yong
>>
>> > 2017-08-16 6:45 GMT-07:00 Andrew Jeffery :
>> > Hi Yong,
>> >
>> > On Wed, 2017-08-16 at 00:21 +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
>> >
>> > ./scripts/checkpatch.pl complains about DOS line-endings thoughout -
>> > you should fix your editor to use Unix line endings (at least for
>> > kernel work). Maybe if Linus is feeling charitable he can fix it up for
>> > you. Regarding the meat of the change:
>> >
>> > > > Reviewed-by: Andrew Jeffery
>> > > > Tested-by: Andrew Jeffery
>> >
>> > Thanks for the patch!
>> >
>> > Andrew
>> >
>> > > ---
>> > > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
>> > > drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
>> > > 2 files changed, 18 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> > > b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> > > index a86a4d6..f2d5133 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 = >descs[i];
>> > >
>> > > @@ -213,8 +214,22 @@ 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) {
>> > > > + ret = regmap_read(maps[ASPEED_IP_SCU],
>> > > > + HW_REVISION_ID, _id);
>> > > > + if (ret < 0)
>> > > > + return ret;
>> > >
>> > > +
>> > > > + if (0x04 == ((rev_id >> 24) & 0xff))
>> > > > + ret = regmap_write(maps[desc->ip],
>> > > > + HW_REVISION_ID, (~val &
>> > > > desc->mask));
>> > > > + else
>
> Looking back at v2 what you had was correct for the single-bit field case (if
> 'val == 0' was false we went and wrote the set bit to HW_STRAP1), and it seems
> I didn't think through my suggestion enough to catch the problem I created.
>
> Essentially we should drop the 'else' above and always perform the
> regmap_update_bits() HW_STRAP1 because it is write-1-set. Otherwise we can
> only clear bits in the strapping register on the AST2500 and not set them.
> However, given the duplication pointed out below, I've suggested a further
> rearrangement.
>
>> > > > + ret = regmap_update_bits(maps[desc->ip],
>> > >
Re: [PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
Hi Andrew,
Thanks for your review. I checked the patch before I sent out, but the
tool did not report any problems. Could you help to share your
checking commands?
scripts/checkpatch.pl
0001-pinctrl-aspeed-Fix-ast2500-strap-register-write-logi.patch
total: 0 errors, 0 warnings, 38 lines checked
Thanks,
Yong
2017-08-16 6:45 GMT-07:00 Andrew Jeffery <and...@aj.id.au>:
> Hi Yong,
>
> On Wed, 2017-08-16 at 00:21 +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 <sdliy...@gmail.com>
>
> ./scripts/checkpatch.pl complains about DOS line-endings thoughout -
> you should fix your editor to use Unix line endings (at least for
> kernel work). Maybe if Linus is feeling charitable he can fix it up for
> you. Regarding the meat of the change:
>
> Reviewed-by: Andrew Jeffery <and...@aj.id.au>
> Tested-by: Andrew Jeffery <and...@aj.id.au>
>
> Thanks for the patch!
>
> Andrew
>
>> ---
>> drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
>> drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> index a86a4d6..f2d5133 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 = >descs[i];
>> @@ -213,8 +214,22 @@ 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) {
>> > + ret = regmap_read(maps[ASPEED_IP_SCU],
>> > + HW_REVISION_ID, _id);
>> > + if (ret < 0)
>> > + return ret;
>> +
>> > + if (0x04 == ((rev_id >> 24) & 0xff))
>> > + ret = regmap_write(maps[desc->ip],
>> > + HW_REVISION_ID, (~val & 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 */
Re: [PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
Hi Andrew,
Thanks for your review. I checked the patch before I sent out, but the
tool did not report any problems. Could you help to share your
checking commands?
scripts/checkpatch.pl
0001-pinctrl-aspeed-Fix-ast2500-strap-register-write-logi.patch
total: 0 errors, 0 warnings, 38 lines checked
Thanks,
Yong
2017-08-16 6:45 GMT-07:00 Andrew Jeffery :
> Hi Yong,
>
> On Wed, 2017-08-16 at 00:21 +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
>
> ./scripts/checkpatch.pl complains about DOS line-endings thoughout -
> you should fix your editor to use Unix line endings (at least for
> kernel work). Maybe if Linus is feeling charitable he can fix it up for
> you. Regarding the meat of the change:
>
> Reviewed-by: Andrew Jeffery
> Tested-by: Andrew Jeffery
>
> Thanks for the patch!
>
> Andrew
>
>> ---
>> drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
>> drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
>> 2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> index a86a4d6..f2d5133 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 = >descs[i];
>> @@ -213,8 +214,22 @@ 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) {
>> > + ret = regmap_read(maps[ASPEED_IP_SCU],
>> > + HW_REVISION_ID, _id);
>> > + if (ret < 0)
>> > + return ret;
>> +
>> > + if (0x04 == ((rev_id >> 24) & 0xff))
>> > + ret = regmap_write(maps[desc->ip],
>> > + HW_REVISION_ID, (~val & 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 */
[PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
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 <sdliy...@gmail.com>
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index a86a4d6..f2d5133 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 = >descs[i];
@@ -213,8 +214,22 @@ 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) {
+ ret = regmap_read(maps[ASPEED_IP_SCU],
+ HW_REVISION_ID, _id);
+ if (ret < 0)
+ return ret;
+
+ if (0x04 == ((rev_id >> 24) & 0xff))
+ ret = regmap_write(maps[desc->ip],
+ HW_REVISION_ID, (~val & 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
[PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic
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
---
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +--
drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 +
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index a86a4d6..f2d5133 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 = >descs[i];
@@ -213,8 +214,22 @@ 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) {
+ ret = regmap_read(maps[ASPEED_IP_SCU],
+ HW_REVISION_ID, _id);
+ if (ret < 0)
+ return ret;
+
+ if (0x04 == ((rev_id >> 24) & 0xff))
+ ret = regmap_write(maps[desc->ip],
+ HW_REVISION_ID, (~val & 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
[PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic
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 <sdliy...@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 = >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, _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
[PATCH v2] pinctrl: aspeed: Fix ast2500 strap register write logic
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
---
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 = >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, _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
[PATCH] pinctrl: aspeed: Fix hardware strap register write logic
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 <sdliy...@gmail.com> --- drivers/pinctrl/aspeed/pinctrl-aspeed.c | 9 +++-- drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c index a86a4d6..4305052 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c @@ -213,8 +213,13 @@ 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); + if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 && + val == 0) + 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); 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
[PATCH] pinctrl: aspeed: Fix hardware strap register write logic
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 --- drivers/pinctrl/aspeed/pinctrl-aspeed.c | 9 +++-- drivers/pinctrl/aspeed/pinctrl-aspeed.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c index a86a4d6..4305052 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c @@ -213,8 +213,13 @@ 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); + if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 && + val == 0) + 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); 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
boot hang issue on S5PV210 with the latest kernel
Hi all, I am testing the latest kernel on my S5PV210 ARM boards. I found I have to add the kputc debug in __armv7_mmu_cache_on in boot/compressed/head.S: mov r11, r3 kputc #'x' mov r3, r11 Without the above kputc, the kernel boot hang, there is no any output on UART Any suggestions? Thanks, Yong
boot hang issue on S5PV210 with the latest kernel
Hi all, I am testing the latest kernel on my S5PV210 ARM boards. I found I have to add the kputc debug in __armv7_mmu_cache_on in boot/compressed/head.S: mov r11, r3 kputc #'x' mov r3, r11 Without the above kputc, the kernel boot hang, there is no any output on UART Any suggestions? Thanks, Yong
[PATCH] ARM: dts: am335x-boneblack: add i2c1 DT entry
From: Yong Li <sdliy...@gmail.com>
Without this patch, I2C-1 is missing on beaglebone black boards
Signed-off-by: Yong Li <sdliy...@gmail.com>
---
arch/arm/boot/dts/am335x-boneblack.dts | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 55c0e95..7eee5c4 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -64,6 +64,13 @@
AM33XX_IOPAD(0x9b0, PIN_OUTPUT_PULLDOWN | MUX_MODE3)
/* xdma_event_intr0 */
>;
};
+
+ i2c1_pins: pinmux_i2c1_pins {
+ pinctrl-single,pins = <
+ AM33XX_IOPAD(0x958, PIN_INPUT_PULLUP | MUX_MODE2)
/* spi0_d1.i2c1_sda */
+ AM33XX_IOPAD(0x95c, PIN_INPUT_PULLUP | MUX_MODE2)
/* spi0_cs0.i2c1_scl */
+ >;
+ };
};
{
@@ -91,6 +98,14 @@
};
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+
+ status = "okay";
+ clock-frequency = <10>;
+};
+
{
system-power-controller;
};
--
2.7.4
[PATCH] ARM: dts: am335x-boneblack: add i2c1 DT entry
From: Yong Li
Without this patch, I2C-1 is missing on beaglebone black boards
Signed-off-by: Yong Li
---
arch/arm/boot/dts/am335x-boneblack.dts | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 55c0e95..7eee5c4 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -64,6 +64,13 @@
AM33XX_IOPAD(0x9b0, PIN_OUTPUT_PULLDOWN | MUX_MODE3)
/* xdma_event_intr0 */
>;
};
+
+ i2c1_pins: pinmux_i2c1_pins {
+ pinctrl-single,pins = <
+ AM33XX_IOPAD(0x958, PIN_INPUT_PULLUP | MUX_MODE2)
/* spi0_d1.i2c1_sda */
+ AM33XX_IOPAD(0x95c, PIN_INPUT_PULLUP | MUX_MODE2)
/* spi0_cs0.i2c1_scl */
+ >;
+ };
};
{
@@ -91,6 +98,14 @@
};
};
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+
+ status = "okay";
+ clock-frequency = <10>;
+};
+
{
system-power-controller;
};
--
2.7.4
[PATCH] iio: light apds9960: Add the missing dev.parent
Without this, the iio:deviceX is missing in the /sys/bus/i2c/devices/0-0039 Signed-off-by: Yong Li <sdliy...@gmail.com> --- drivers/iio/light/apds9960.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c index 35928fb..f47cc0a 100644 --- a/drivers/iio/light/apds9960.c +++ b/drivers/iio/light/apds9960.c @@ -1010,6 +1010,7 @@ static int apds9960_probe(struct i2c_client *client, iio_device_attach_buffer(indio_dev, buffer); + indio_dev->dev.parent = >dev; indio_dev->info = _info; indio_dev->name = APDS9960_DRV_NAME; indio_dev->channels = apds9960_channels; -- 2.5.0
[PATCH] iio: light apds9960: Add the missing dev.parent
Without this, the iio:deviceX is missing in the /sys/bus/i2c/devices/0-0039 Signed-off-by: Yong Li --- drivers/iio/light/apds9960.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c index 35928fb..f47cc0a 100644 --- a/drivers/iio/light/apds9960.c +++ b/drivers/iio/light/apds9960.c @@ -1010,6 +1010,7 @@ static int apds9960_probe(struct i2c_client *client, iio_device_attach_buffer(indio_dev, buffer); + indio_dev->dev.parent = >dev; indio_dev->info = _info; indio_dev->name = APDS9960_DRV_NAME; indio_dev->channels = apds9960_channels; -- 2.5.0
Re: [PATCH] iio: tmp006: Set correct iio name
Thanks for your mails. Is it possible to just merge this patch, then test if there is any application is using it? Considering almost all other I2C devices are using the correct ID name, it should be low risky Yong 2016-04-26 23:21 GMT+08:00 Daniel Baluta <daniel.bal...@gmail.com>: > On Tue, Apr 26, 2016 at 4:14 PM, Yong Li <sdliy...@gmail.com> wrote: >> I am thinking if there is any application is using this incorrect >> name, the application should be fix too > > The rule is: "Don't break the userspace ABI". So, if we got this wrong > from the beginning we are stuck with this name. > > The only thing that can save the situation is to know that there is no > application relying on the name :).
Re: [PATCH] iio: tmp006: Set correct iio name
Thanks for your mails. Is it possible to just merge this patch, then test if there is any application is using it? Considering almost all other I2C devices are using the correct ID name, it should be low risky Yong 2016-04-26 23:21 GMT+08:00 Daniel Baluta : > On Tue, Apr 26, 2016 at 4:14 PM, Yong Li wrote: >> I am thinking if there is any application is using this incorrect >> name, the application should be fix too > > The rule is: "Don't break the userspace ABI". So, if we got this wrong > from the beginning we are stuck with this name. > > The only thing that can save the situation is to know that there is no > application relying on the name :).
Re: [PATCH] iio: tmp006: Set correct iio name
I am thinking if there is any application is using this incorrect name, the application should be fix too Thanks, Yong 2016-04-26 20:01 GMT+08:00 Lars-Peter Clausen <l...@metafoo.de>: > On 04/26/2016 01:47 PM, Yong Li wrote: >> Thanks for your mails. Just FYI, we are testing this tmp006 sensor on >> our system. and our application framework is using these device names, >> so I submitted this patch. > > Your patch is certainly correct. But the problem is, or rather the question > is, will this break any existing applications that rely on the wrong behavior? > > - Lars
Re: [PATCH] iio: tmp006: Set correct iio name
I am thinking if there is any application is using this incorrect name, the application should be fix too Thanks, Yong 2016-04-26 20:01 GMT+08:00 Lars-Peter Clausen : > On 04/26/2016 01:47 PM, Yong Li wrote: >> Thanks for your mails. Just FYI, we are testing this tmp006 sensor on >> our system. and our application framework is using these device names, >> so I submitted this patch. > > Your patch is certainly correct. But the problem is, or rather the question > is, will this break any existing applications that rely on the wrong behavior? > > - Lars
Re: [PATCH] iio: tmp006: Set correct iio name
Thanks for your mails. Just FYI, we are testing this tmp006 sensor on our system. and our application framework is using these device names, so I submitted this patch. Yong 2016-04-26 18:57 GMT+08:00 Lars-Peter Clausen <l...@metafoo.de>: > On 04/25/2016 11:11 PM, Jonathan Cameron wrote: >> On 25/04/16 21:59, Crestez Dan Leonard wrote: >>> On 04/25/2016 10:33 PM, Jonathan Cameron wrote: >>>> On 22/04/16 04:43, Yong Li wrote: >>>>> When load the driver using the below command: >>>>> echo tmp006 0x40 > /sys/bus/i2c/devices/i2c-0/new_device >>>>> >>>>> In sysfs, the i2c name is tmp006, however the iio name is 0-0040, >>>>> they are inconsistent. With this patch, >>>>> the iio name will be the same as the i2c device name >>>>> >>>>> Signed-off-by: Yong Li <sdliy...@gmail.com> >>>> Peter, this looks right to me, but could you take a quick look as I guess >>>> there might be a reason you did this in an unusual way originally? >>>> >>> Is there a "correct" or "usual" way to set indio_dev->name? Some quick >>> grepping shows no clear standard: >>> >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | wc -l >>> 148 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep id | wc -l >>> 52 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep dev_name | wc -l >>> 20 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i drv | wc -l >>> 19 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i driver | wc -l >>> 15 >>> >>> It seems that many devices use dev_name(_client->dev) or >>> otherwise some sort of "ABC123_DRIVER_NAME" constant. >>> >>> It's also not clear what this "name" field is for. Is it more than >>> just a cosmetic sysfs attribute? It seems to me that names don't have >>> to be unique so it would be wrong to use them for identification. >>> >> It's a convenience field really as there is no clear standard for where else >> to find out what a part actually is (i.e. if it is an i2c part you can look >> in the name attribute i2c supplies - but otherwise you are on your own). > > I'd like to argue that it is supposed to be the device type allowing the > application to identify which kind of device they are talking to. In which > case the dev_name() initialization is wrong. Unfortunately there seem quite > a few drivers which use it, especially the the SoC ADC drivers. But we can't > really change this for existing drivers since there might be applications > relying on the name. > > We should pay more attention to this for new driver submissions and make > sure we don't get any more of this. > > As a side note if you want to know the dev_name() of the parent device you > can do a readlink() on the iio device in /sys/bus/iio/devices/. The second > last entry in the path name is the name of the parent device.
Re: [PATCH] iio: tmp006: Set correct iio name
Thanks for your mails. Just FYI, we are testing this tmp006 sensor on our system. and our application framework is using these device names, so I submitted this patch. Yong 2016-04-26 18:57 GMT+08:00 Lars-Peter Clausen : > On 04/25/2016 11:11 PM, Jonathan Cameron wrote: >> On 25/04/16 21:59, Crestez Dan Leonard wrote: >>> On 04/25/2016 10:33 PM, Jonathan Cameron wrote: >>>> On 22/04/16 04:43, Yong Li wrote: >>>>> When load the driver using the below command: >>>>> echo tmp006 0x40 > /sys/bus/i2c/devices/i2c-0/new_device >>>>> >>>>> In sysfs, the i2c name is tmp006, however the iio name is 0-0040, >>>>> they are inconsistent. With this patch, >>>>> the iio name will be the same as the i2c device name >>>>> >>>>> Signed-off-by: Yong Li >>>> Peter, this looks right to me, but could you take a quick look as I guess >>>> there might be a reason you did this in an unusual way originally? >>>> >>> Is there a "correct" or "usual" way to set indio_dev->name? Some quick >>> grepping shows no clear standard: >>> >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | wc -l >>> 148 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep id | wc -l >>> 52 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep dev_name | wc -l >>> 20 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i drv | wc -l >>> 19 >>> $ git grep -h 'indio_dev->name =' drivers/iio/ | grep -i driver | wc -l >>> 15 >>> >>> It seems that many devices use dev_name(_client->dev) or >>> otherwise some sort of "ABC123_DRIVER_NAME" constant. >>> >>> It's also not clear what this "name" field is for. Is it more than >>> just a cosmetic sysfs attribute? It seems to me that names don't have >>> to be unique so it would be wrong to use them for identification. >>> >> It's a convenience field really as there is no clear standard for where else >> to find out what a part actually is (i.e. if it is an i2c part you can look >> in the name attribute i2c supplies - but otherwise you are on your own). > > I'd like to argue that it is supposed to be the device type allowing the > application to identify which kind of device they are talking to. In which > case the dev_name() initialization is wrong. Unfortunately there seem quite > a few drivers which use it, especially the the SoC ADC drivers. But we can't > really change this for existing drivers since there might be applications > relying on the name. > > We should pay more attention to this for new driver submissions and make > sure we don't get any more of this. > > As a side note if you want to know the dev_name() of the parent device you > can do a readlink() on the iio device in /sys/bus/iio/devices/. The second > last entry in the path name is the name of the parent device.
[PATCH] iio: tmp006: Set correct iio name
When load the driver using the below command: echo tmp006 0x40 > /sys/bus/i2c/devices/i2c-0/new_device In sysfs, the i2c name is tmp006, however the iio name is 0-0040, they are inconsistent. With this patch, the iio name will be the same as the i2c device name Signed-off-by: Yong Li <sdliy...@gmail.com> --- drivers/iio/temperature/tmp006.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/temperature/tmp006.c b/drivers/iio/temperature/tmp006.c index 18c9b43..17c8413 100644 --- a/drivers/iio/temperature/tmp006.c +++ b/drivers/iio/temperature/tmp006.c @@ -221,7 +221,7 @@ static int tmp006_probe(struct i2c_client *client, data->client = client; indio_dev->dev.parent = >dev; - indio_dev->name = dev_name(>dev); + indio_dev->name = id->name; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->info = _info; -- 2.5.0
[PATCH] iio: tmp006: Set correct iio name
When load the driver using the below command: echo tmp006 0x40 > /sys/bus/i2c/devices/i2c-0/new_device In sysfs, the i2c name is tmp006, however the iio name is 0-0040, they are inconsistent. With this patch, the iio name will be the same as the i2c device name Signed-off-by: Yong Li --- drivers/iio/temperature/tmp006.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/temperature/tmp006.c b/drivers/iio/temperature/tmp006.c index 18c9b43..17c8413 100644 --- a/drivers/iio/temperature/tmp006.c +++ b/drivers/iio/temperature/tmp006.c @@ -221,7 +221,7 @@ static int tmp006_probe(struct i2c_client *client, data->client = client; indio_dev->dev.parent = >dev; - indio_dev->name = dev_name(>dev); + indio_dev->name = id->name; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->info = _info; -- 2.5.0
[PATCH v2] gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2
Galileo Gen2 board uses the PCAL9535 as the GPIO expansion,
it is different from PCA9535 and includes interrupt mask/status registers,
The current driver does not support the interrupt registers configuration,
it causes some gpio pins cannot trigger interrupt events,
this patch fix this issue. The original patch was submitted by
Josef Ahmad <josef.ah...@linux.intel.com>
http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-kernel/linux/files/0015-Quark-GPIO-1-2-quark.patch
Signed-off-by: Yong Li <yong.b...@intel.com>
---
drivers/gpio/gpio-pca953x.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e66084c..5e3be32 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -38,8 +38,13 @@
#define PCA957X_MSK6
#define PCA957X_INTS 7
+#define PCAL953X_IN_LATCH 34
+#define PCAL953X_INT_MASK 37
+#define PCAL953X_INT_STAT 38
+
#define PCA_GPIO_MASK 0x00FF
#define PCA_INT0x0100
+#define PCA_PCAL 0x0200
#define PCA953X_TYPE 0x1000
#define PCA957X_TYPE 0x2000
#define PCA_TYPE_MASK 0xF000
@@ -77,7 +82,7 @@ static const struct i2c_device_id pca953x_id[] = {
MODULE_DEVICE_TABLE(i2c, pca953x_id);
static const struct acpi_device_id pca953x_acpi_ids[] = {
- { "INT3491", 16 | PCA953X_TYPE | PCA_INT, },
+ { "INT3491", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, },
{ }
};
MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
@@ -437,6 +442,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 new_irqs;
int level, i;
+ u8 invert_irq_mask[MAX_BANK];
+
+ if (chip->driver_data & PCA_PCAL) {
+ /* Enable latch on interrupt-enabled inputs */
+ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
+
+ for (i = 0; i < NBANK(chip); i++)
+ invert_irq_mask[i] = ~chip->irq_mask[i];
+
+ /* Unmask enabled interrupts */
+ pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask);
+ }
/* Look for any newly setup interrupt */
for (i = 0; i < NBANK(chip); i++) {
@@ -498,6 +515,29 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip,
u8 *pending)
u8 trigger[MAX_BANK];
int ret, i, offset = 0;
+ if (chip->driver_data & PCA_PCAL) {
+ /* Read the current interrupt status from the device */
+ ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
+ if (ret)
+ return false;
+
+ /* Check latched inputs and clear interrupt status */
+ ret = pca953x_read_regs(chip, PCA953X_INPUT, cur_stat);
+ if (ret)
+ return false;
+
+ for (i = 0; i < NBANK(chip); i++) {
+ /* Apply filter for rising/falling edge selection */
+ pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) |
+ (cur_stat[i] & chip->irq_trig_raise[i]);
+ pending[i] &= trigger[i];
+ if (pending[i])
+ pending_seen = true;
+ }
+
+ return pending_seen;
+ }
+
switch (chip->chip_type) {
case PCA953X_TYPE:
offset = PCA953X_INPUT;
--
2.1.4
[PATCH v2] gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2
Galileo Gen2 board uses the PCAL9535 as the GPIO expansion,
it is different from PCA9535 and includes interrupt mask/status registers,
The current driver does not support the interrupt registers configuration,
it causes some gpio pins cannot trigger interrupt events,
this patch fix this issue. The original patch was submitted by
Josef Ahmad
http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-kernel/linux/files/0015-Quark-GPIO-1-2-quark.patch
Signed-off-by: Yong Li
---
drivers/gpio/gpio-pca953x.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e66084c..5e3be32 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -38,8 +38,13 @@
#define PCA957X_MSK6
#define PCA957X_INTS 7
+#define PCAL953X_IN_LATCH 34
+#define PCAL953X_INT_MASK 37
+#define PCAL953X_INT_STAT 38
+
#define PCA_GPIO_MASK 0x00FF
#define PCA_INT0x0100
+#define PCA_PCAL 0x0200
#define PCA953X_TYPE 0x1000
#define PCA957X_TYPE 0x2000
#define PCA_TYPE_MASK 0xF000
@@ -77,7 +82,7 @@ static const struct i2c_device_id pca953x_id[] = {
MODULE_DEVICE_TABLE(i2c, pca953x_id);
static const struct acpi_device_id pca953x_acpi_ids[] = {
- { "INT3491", 16 | PCA953X_TYPE | PCA_INT, },
+ { "INT3491", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, },
{ }
};
MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
@@ -437,6 +442,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 new_irqs;
int level, i;
+ u8 invert_irq_mask[MAX_BANK];
+
+ if (chip->driver_data & PCA_PCAL) {
+ /* Enable latch on interrupt-enabled inputs */
+ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
+
+ for (i = 0; i < NBANK(chip); i++)
+ invert_irq_mask[i] = ~chip->irq_mask[i];
+
+ /* Unmask enabled interrupts */
+ pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask);
+ }
/* Look for any newly setup interrupt */
for (i = 0; i < NBANK(chip); i++) {
@@ -498,6 +515,29 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip,
u8 *pending)
u8 trigger[MAX_BANK];
int ret, i, offset = 0;
+ if (chip->driver_data & PCA_PCAL) {
+ /* Read the current interrupt status from the device */
+ ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
+ if (ret)
+ return false;
+
+ /* Check latched inputs and clear interrupt status */
+ ret = pca953x_read_regs(chip, PCA953X_INPUT, cur_stat);
+ if (ret)
+ return false;
+
+ for (i = 0; i < NBANK(chip); i++) {
+ /* Apply filter for rising/falling edge selection */
+ pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) |
+ (cur_stat[i] & chip->irq_trig_raise[i]);
+ pending[i] &= trigger[i];
+ if (pending[i])
+ pending_seen = true;
+ }
+
+ return pending_seen;
+ }
+
switch (chip->chip_type) {
case PCA953X_TYPE:
offset = PCA953X_INPUT;
--
2.1.4
[PATCH] gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2
Galileo Gen2 board uses the PCAL9535 as the GPIO expansion,
it is different from PCA9535 and includes interrupt mask/status registers,
The current driver does not support the interrupt registers configuration,
it causes some gpio pins cannot trigger interrupt events,
this patch fix this issue. The original patch was submitted by
Josef Ahmad <josef.ah...@linux.intel.com>
http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-kernel/linux/files/0015-Quark-GPIO-1-2-quark.patch
Signed-off-by: Yong Li <yong.b...@intel.com>
---
drivers/gpio/gpio-pca953x.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e66084c..cf2f73b 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -38,8 +38,13 @@
#define PCA957X_MSK6
#define PCA957X_INTS 7
+#define PCAL953X_IN_LATCH 34
+#define PCAL953X_INT_MASK 37
+#define PCAL953X_INT_STAT 38
+
#define PCA_GPIO_MASK 0x00FF
#define PCA_INT0x0100
+#define PCA_PCAL 0x0200
#define PCA953X_TYPE 0x1000
#define PCA957X_TYPE 0x2000
#define PCA_TYPE_MASK 0xF000
@@ -77,7 +82,7 @@ static const struct i2c_device_id pca953x_id[] = {
MODULE_DEVICE_TABLE(i2c, pca953x_id);
static const struct acpi_device_id pca953x_acpi_ids[] = {
- { "INT3491", 16 | PCA953X_TYPE | PCA_INT, },
+ { "INT3491", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, },
{ }
};
MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
@@ -437,6 +442,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 new_irqs;
int level, i;
+ u8 invert_irq_mask[MAX_BANK];
+
+ if (chip->driver_data & PCA_PCAL) {
+ /* Enable latch on interrupt-enabled inputs */
+ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
+
+ for (i = 0; i < NBANK(chip); i++)
+ invert_irq_mask[i] = ~chip->irq_mask[i];
+
+ /* Unmask enabled interrupts */
+ pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask);
+ }
/* Look for any newly setup interrupt */
for (i = 0; i < NBANK(chip); i++) {
@@ -498,6 +515,29 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip,
u8 *pending)
u8 trigger[MAX_BANK];
int ret, i, offset = 0;
+ if (chip->driver_data & PCA_PCAL) {
+ /* Read the current interrupt status from the device */
+ ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
+ if (ret)
+ return 0;
+
+ /* Check latched inputs and clear interrupt status */
+ ret = pca953x_read_regs(chip, PCA953X_INPUT, cur_stat);
+ if (ret)
+ return 0;
+
+ for (i = 0; i < NBANK(chip); i++) {
+ /* Apply filter for rising/falling edge selection */
+ pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) |
+ (cur_stat[i] & chip->irq_trig_raise[i]);
+ pending[i] &= trigger[i];
+ if (pending[i])
+ pending_seen = true;
+ }
+
+ return pending_seen;
+ }
+
switch (chip->chip_type) {
case PCA953X_TYPE:
offset = PCA953X_INPUT;
--
2.1.4
[PATCH] gpio: pca953x: add PCAL9535 interrupt support for Galileo Gen2
Galileo Gen2 board uses the PCAL9535 as the GPIO expansion,
it is different from PCA9535 and includes interrupt mask/status registers,
The current driver does not support the interrupt registers configuration,
it causes some gpio pins cannot trigger interrupt events,
this patch fix this issue. The original patch was submitted by
Josef Ahmad
http://git.yoctoproject.org/cgit/cgit.cgi/meta-intel-quark/tree/recipes-kernel/linux/files/0015-Quark-GPIO-1-2-quark.patch
Signed-off-by: Yong Li
---
drivers/gpio/gpio-pca953x.c | 42 +-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e66084c..cf2f73b 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -38,8 +38,13 @@
#define PCA957X_MSK6
#define PCA957X_INTS 7
+#define PCAL953X_IN_LATCH 34
+#define PCAL953X_INT_MASK 37
+#define PCAL953X_INT_STAT 38
+
#define PCA_GPIO_MASK 0x00FF
#define PCA_INT0x0100
+#define PCA_PCAL 0x0200
#define PCA953X_TYPE 0x1000
#define PCA957X_TYPE 0x2000
#define PCA_TYPE_MASK 0xF000
@@ -77,7 +82,7 @@ static const struct i2c_device_id pca953x_id[] = {
MODULE_DEVICE_TABLE(i2c, pca953x_id);
static const struct acpi_device_id pca953x_acpi_ids[] = {
- { "INT3491", 16 | PCA953X_TYPE | PCA_INT, },
+ { "INT3491", 16 | PCA953X_TYPE | PCA_INT | PCA_PCAL, },
{ }
};
MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
@@ -437,6 +442,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 new_irqs;
int level, i;
+ u8 invert_irq_mask[MAX_BANK];
+
+ if (chip->driver_data & PCA_PCAL) {
+ /* Enable latch on interrupt-enabled inputs */
+ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
+
+ for (i = 0; i < NBANK(chip); i++)
+ invert_irq_mask[i] = ~chip->irq_mask[i];
+
+ /* Unmask enabled interrupts */
+ pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask);
+ }
/* Look for any newly setup interrupt */
for (i = 0; i < NBANK(chip); i++) {
@@ -498,6 +515,29 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip,
u8 *pending)
u8 trigger[MAX_BANK];
int ret, i, offset = 0;
+ if (chip->driver_data & PCA_PCAL) {
+ /* Read the current interrupt status from the device */
+ ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
+ if (ret)
+ return 0;
+
+ /* Check latched inputs and clear interrupt status */
+ ret = pca953x_read_regs(chip, PCA953X_INPUT, cur_stat);
+ if (ret)
+ return 0;
+
+ for (i = 0; i < NBANK(chip); i++) {
+ /* Apply filter for rising/falling edge selection */
+ pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) |
+ (cur_stat[i] & chip->irq_trig_raise[i]);
+ pending[i] &= trigger[i];
+ if (pending[i])
+ pending_seen = true;
+ }
+
+ return pending_seen;
+ }
+
switch (chip->chip_type) {
case PCA953X_TYPE:
offset = PCA953X_INPUT;
--
2.1.4
Re: [PATCH v2] gpio: pca953x: Use correct u16 value for register word write
Yes it is a regression. I think it should be tagged for stable too Thanks, Yong 2016-04-01 20:51 GMT+08:00 Linus Walleij <linus.wall...@linaro.org>: > On Wed, Mar 30, 2016 at 8:49 AM, Yong Li <sdliy...@gmail.com> wrote: > >> The current implementation only uses the first byte in val, >> the second byte is always 0. Change it to use cpu_to_le16 >> to write the two bytes into the register >> >> Signed-off-by: Yong Li <sdliy...@gmail.com> > > Patch applied to fixes with Phil's review tag. > > It is a regression, right? > > Should it be tagged for stable? > > Yours, > Linus Walleij
Re: [PATCH v2] gpio: pca953x: Use correct u16 value for register word write
Yes it is a regression. I think it should be tagged for stable too Thanks, Yong 2016-04-01 20:51 GMT+08:00 Linus Walleij : > On Wed, Mar 30, 2016 at 8:49 AM, Yong Li wrote: > >> The current implementation only uses the first byte in val, >> the second byte is always 0. Change it to use cpu_to_le16 >> to write the two bytes into the register >> >> Signed-off-by: Yong Li > > Patch applied to fixes with Phil's review tag. > > It is a regression, right? > > Should it be tagged for stable? > > Yours, > Linus Walleij
[PATCH v2] gpio: pca953x: Use correct u16 value for register word write
The current implementation only uses the first byte in val,
the second byte is always 0. Change it to use cpu_to_le16
to write the two bytes into the register
Signed-off-by: Yong Li <sdliy...@gmail.com>
---
drivers/gpio/gpio-pca953x.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d0d3065..e66084c 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -159,7 +160,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip,
int reg, u8 *val)
switch (chip->chip_type) {
case PCA953X_TYPE:
ret = i2c_smbus_write_word_data(chip->client,
- reg << 1, (u16) *val);
+ reg << 1, cpu_to_le16(get_unaligned((u16 *)val)));
break;
case PCA957X_TYPE:
ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
--
2.1.4
[PATCH v2] gpio: pca953x: Use correct u16 value for register word write
The current implementation only uses the first byte in val,
the second byte is always 0. Change it to use cpu_to_le16
to write the two bytes into the register
Signed-off-by: Yong Li
---
drivers/gpio/gpio-pca953x.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d0d3065..e66084c 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -159,7 +160,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip,
int reg, u8 *val)
switch (chip->chip_type) {
case PCA953X_TYPE:
ret = i2c_smbus_write_word_data(chip->client,
- reg << 1, (u16) *val);
+ reg << 1, cpu_to_le16(get_unaligned((u16 *)val)));
break;
case PCA957X_TYPE:
ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
--
2.1.4
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
Or another method is using the below to convert the u8 to u16: cpu_to_le16(get_unaligned((u16 *) val)), compared with the i2c_smbus_write_byte_data method, which one is better? Thanks, Yong 2016-03-30 10:43 GMT+08:00 Yong Li <sdliy...@gmail.com>: > If use the get_unaligned, below is the code example, but we cannot detect if > it is big endian or little endian. I would like to use the same write logic > as PCA957X_TYPE: use the i2c_smbus_write_byte_data API to write two times. > How do you think about it? > > if (big_endian) > value = get_unaligned_be16(buf); > else > value = get_unaligned_le16(buf); > > Thanks, > Yong Li > 2016-03-30 0:33 GMT+08:00 Phil Reid <pr...@electromag.com.au>: >> >> On 29/03/2016 10:39 PM, Alexander Stein wrote: >>> >>> You missed CC'ing Phil (Added for this post) >>> >>> On Tuesday 29 March 2016 20:53:58, Yong Li wrote: >>>> >>>> Thanks for your comment, I think I can change it to val[0] | (val[1] >>>> << 8), is it okay ? >>> >>> >>> Mh, currently there is only one caller (device_pca953x_init) which passes >>> only >>> 0, 0 or 0xff, 0xff, so endianess is irrelevant. But to be future proof >>> this >>> should be done in an endian-safe manner. Though cpu_to_le16p does not >>> work, >>> due to same alignment problem as casting to u16*. >>> >> >> I think get_unaligned((u16 *) val) should do the job. >> There's also get_unaligned_le* get_unaligned_be* >> >> -- >> Regards >> Phil Reid >>
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
Or another method is using the below to convert the u8 to u16: cpu_to_le16(get_unaligned((u16 *) val)), compared with the i2c_smbus_write_byte_data method, which one is better? Thanks, Yong 2016-03-30 10:43 GMT+08:00 Yong Li : > If use the get_unaligned, below is the code example, but we cannot detect if > it is big endian or little endian. I would like to use the same write logic > as PCA957X_TYPE: use the i2c_smbus_write_byte_data API to write two times. > How do you think about it? > > if (big_endian) > value = get_unaligned_be16(buf); > else > value = get_unaligned_le16(buf); > > Thanks, > Yong Li > 2016-03-30 0:33 GMT+08:00 Phil Reid : >> >> On 29/03/2016 10:39 PM, Alexander Stein wrote: >>> >>> You missed CC'ing Phil (Added for this post) >>> >>> On Tuesday 29 March 2016 20:53:58, Yong Li wrote: >>>> >>>> Thanks for your comment, I think I can change it to val[0] | (val[1] >>>> << 8), is it okay ? >>> >>> >>> Mh, currently there is only one caller (device_pca953x_init) which passes >>> only >>> 0, 0 or 0xff, 0xff, so endianess is irrelevant. But to be future proof >>> this >>> should be done in an endian-safe manner. Though cpu_to_le16p does not >>> work, >>> due to same alignment problem as casting to u16*. >>> >> >> I think get_unaligned((u16 *) val) should do the job. >> There's also get_unaligned_le* get_unaligned_be* >> >> -- >> Regards >> Phil Reid >>
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
If use the get_unaligned, below is the code example, but we cannot detect if it is big endian or little endian. I would like to use the same write logic as PCA957X_TYPE: use the i2c_smbus_write_byte_data API to write two times. How do you think about it? if (big_endian) value = get_unaligned_be16(buf); else value = get_unaligned_le16(buf); Thanks, Yong Li 2016-03-30 0:33 GMT+08:00 Phil Reid <pr...@electromag.com.au>: > On 29/03/2016 10:39 PM, Alexander Stein wrote: >> >> You missed CC'ing Phil (Added for this post) >> >> On Tuesday 29 March 2016 20:53:58, Yong Li wrote: >>> >>> Thanks for your comment, I think I can change it to val[0] | (val[1] >>> << 8), is it okay ? >> >> >> Mh, currently there is only one caller (device_pca953x_init) which passes >> only >> 0, 0 or 0xff, 0xff, so endianess is irrelevant. But to be future proof >> this >> should be done in an endian-safe manner. Though cpu_to_le16p does not >> work, >> due to same alignment problem as casting to u16*. >> > > I think get_unaligned((u16 *) val) should do the job. > There's also get_unaligned_le* get_unaligned_be* > > -- > Regards > Phil Reid >
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
If use the get_unaligned, below is the code example, but we cannot detect if it is big endian or little endian. I would like to use the same write logic as PCA957X_TYPE: use the i2c_smbus_write_byte_data API to write two times. How do you think about it? if (big_endian) value = get_unaligned_be16(buf); else value = get_unaligned_le16(buf); Thanks, Yong Li 2016-03-30 0:33 GMT+08:00 Phil Reid : > On 29/03/2016 10:39 PM, Alexander Stein wrote: >> >> You missed CC'ing Phil (Added for this post) >> >> On Tuesday 29 March 2016 20:53:58, Yong Li wrote: >>> >>> Thanks for your comment, I think I can change it to val[0] | (val[1] >>> << 8), is it okay ? >> >> >> Mh, currently there is only one caller (device_pca953x_init) which passes >> only >> 0, 0 or 0xff, 0xff, so endianess is irrelevant. But to be future proof >> this >> should be done in an endian-safe manner. Though cpu_to_le16p does not >> work, >> due to same alignment problem as casting to u16*. >> > > I think get_unaligned((u16 *) val) should do the job. > There's also get_unaligned_le* get_unaligned_be* > > -- > Regards > Phil Reid >
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
Thanks for your comment, I think I can change it to val[0] | (val[1]
<< 8), is it okay ?
2016-03-29 20:06 GMT+08:00 Phil Reid <pr...@electromag.com.au>:
> G'day Yong,
>
> One comment below.
>
> On 29/03/2016 2:27 PM, Yong Li wrote:
>>
>> The current implementation only uses the first byte in *val,
>> the second data is always 0. Change it to *(u16 *)val
>> to write the two bytes into the register
>>
>> Signed-off-by: Yong Li <sdliy...@gmail.com>
>> ---
>> drivers/gpio/gpio-pca953x.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index d0d3065..cf3d410 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -159,7 +159,7 @@ static int pca953x_write_regs(struct pca953x_chip
>> *chip, int reg, u8 *val)
>> switch (chip->chip_type) {
>> case PCA953X_TYPE:
>> ret = i2c_smbus_write_word_data(chip->client,
>> - reg << 1, (u16)
>> *val);
>> + reg << 1, *(u16
>> *)val);
>
> I don't think this is safe for systems that don't support unaligned memory
> access.
>
>
>> break;
>> case PCA957X_TYPE:
>> ret = i2c_smbus_write_byte_data(chip->client, reg
>> << 1,
>>
>
>
> --
> Regards
> Phil Reid
>
Re: [PATCH] gpio: pca953x: Use correct u16 value for register word write
Thanks for your comment, I think I can change it to val[0] | (val[1]
<< 8), is it okay ?
2016-03-29 20:06 GMT+08:00 Phil Reid :
> G'day Yong,
>
> One comment below.
>
> On 29/03/2016 2:27 PM, Yong Li wrote:
>>
>> The current implementation only uses the first byte in *val,
>> the second data is always 0. Change it to *(u16 *)val
>> to write the two bytes into the register
>>
>> Signed-off-by: Yong Li
>> ---
>> drivers/gpio/gpio-pca953x.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index d0d3065..cf3d410 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -159,7 +159,7 @@ static int pca953x_write_regs(struct pca953x_chip
>> *chip, int reg, u8 *val)
>> switch (chip->chip_type) {
>> case PCA953X_TYPE:
>> ret = i2c_smbus_write_word_data(chip->client,
>> - reg << 1, (u16)
>> *val);
>> + reg << 1, *(u16
>> *)val);
>
> I don't think this is safe for systems that don't support unaligned memory
> access.
>
>
>> break;
>> case PCA957X_TYPE:
>> ret = i2c_smbus_write_byte_data(chip->client, reg
>> << 1,
>>
>
>
> --
> Regards
> Phil Reid
>
[PATCH] gpio: pca953x: Use correct u16 value for register word write
The current implementation only uses the first byte in *val,
the second data is always 0. Change it to *(u16 *)val
to write the two bytes into the register
Signed-off-by: Yong Li <sdliy...@gmail.com>
---
drivers/gpio/gpio-pca953x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d0d3065..cf3d410 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -159,7 +159,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip,
int reg, u8 *val)
switch (chip->chip_type) {
case PCA953X_TYPE:
ret = i2c_smbus_write_word_data(chip->client,
- reg << 1, (u16) *val);
+ reg << 1, *(u16 *)val);
break;
case PCA957X_TYPE:
ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
--
2.1.4
[PATCH] gpio: pca953x: Use correct u16 value for register word write
The current implementation only uses the first byte in *val,
the second data is always 0. Change it to *(u16 *)val
to write the two bytes into the register
Signed-off-by: Yong Li
---
drivers/gpio/gpio-pca953x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d0d3065..cf3d410 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -159,7 +159,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip,
int reg, u8 *val)
switch (chip->chip_type) {
case PCA953X_TYPE:
ret = i2c_smbus_write_word_data(chip->client,
- reg << 1, (u16) *val);
+ reg << 1, *(u16 *)val);
break;
case PCA957X_TYPE:
ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
--
2.1.4
[PATCH] iio: dac: mcp4725: set iio name property in sysfs
Without this change, the name entity for mcp4725 is missing in /sys/bus/iio/devices/iio\:device*/name With this change, name is reported correctly Signed-off-by: Yong Li --- drivers/iio/dac/mcp4725.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c index fb4b336..cca935c 100644 --- a/drivers/iio/dac/mcp4725.c +++ b/drivers/iio/dac/mcp4725.c @@ -346,6 +346,7 @@ static int mcp4725_probe(struct i2c_client *client, data->client = client; indio_dev->dev.parent = >dev; + indio_dev->name = id->name; indio_dev->info = _info; indio_dev->channels = _channel[id->driver_data]; indio_dev->num_channels = 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] iio: dac: mcp4725: set iio name property in sysfs
Without this change, the name entity for mcp4725 is missing in /sys/bus/iio/devices/iio\:device*/name With this change, name is reported correctly Signed-off-by: Yong Li <sdliy...@gmail.com> --- drivers/iio/dac/mcp4725.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c index fb4b336..cca935c 100644 --- a/drivers/iio/dac/mcp4725.c +++ b/drivers/iio/dac/mcp4725.c @@ -346,6 +346,7 @@ static int mcp4725_probe(struct i2c_client *client, data->client = client; indio_dev->dev.parent = >dev; + indio_dev->name = id->name; indio_dev->info = _info; indio_dev->channels = _channel[id->driver_data]; indio_dev->num_channels = 1; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: OMAP2+: allow DEBUG_UNCOMPRESS for OMAP2+
Based on the below
commit ae3c99a26c60 ("ARM: 7806/1: allow DEBUG_UNCOMPRESS for Tegra"),
change the .data section to .text section,
to enable DEBUG_UNCOMPRESS for OMAP2+ platforms
Tested okay using BeagleBone Black
Signed-off-by: Yong Li
---
arch/arm/Kconfig.debug | 2 +-
arch/arm/include/debug/omap2plus.S | 13 ++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 259c0ca..e5ae36a 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1596,7 +1596,7 @@ config DEBUG_UART_8250_FLOW_CONTROL
config DEBUG_UNCOMPRESS
bool
depends on ARCH_MULTIPLATFORM || PLAT_SAMSUNG || ARM_SINGLE_ARMV7M
- default y if DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \
+ default y if DEBUG_LL && (!DEBUG_OMAP2PLUS_UART || !ZBOOT_ROM) && \
(!DEBUG_TEGRA_UART || !ZBOOT_ROM)
help
This option influences the normal decompressor output for
diff --git a/arch/arm/include/debug/omap2plus.S
b/arch/arm/include/debug/omap2plus.S
index 6d867ae..12421f9 100644
--- a/arch/arm/include/debug/omap2plus.S
+++ b/arch/arm/include/debug/omap2plus.S
@@ -58,11 +58,18 @@
#define UART_OFFSET(addr) ((addr) & 0x00ff)
+#if defined(ZIMAGE)
+ omap_uart_phys: .word 0
+ omap_uart_virt: .word 0
+ omap_uart_lsr: .word 0
+#else
+
.pushsection .data
-omap_uart_phys:.word 0
-omap_uart_virt:.word 0
-omap_uart_lsr: .word 0
+ omap_uart_phys: .word 0
+ omap_uart_virt: .word 0
+ omap_uart_lsr: .word 0
.popsection
+#endif
.macro addruart, rp, rv, tmp
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: OMAP2+: allow DEBUG_UNCOMPRESS for OMAP2+
Based on the below
commit ae3c99a26c60 ("ARM: 7806/1: allow DEBUG_UNCOMPRESS for Tegra"),
change the .data section to .text section,
to enable DEBUG_UNCOMPRESS for OMAP2+ platforms
Tested okay using BeagleBone Black
Signed-off-by: Yong Li <sdliy...@gmail.com>
---
arch/arm/Kconfig.debug | 2 +-
arch/arm/include/debug/omap2plus.S | 13 ++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index 259c0ca..e5ae36a 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1596,7 +1596,7 @@ config DEBUG_UART_8250_FLOW_CONTROL
config DEBUG_UNCOMPRESS
bool
depends on ARCH_MULTIPLATFORM || PLAT_SAMSUNG || ARM_SINGLE_ARMV7M
- default y if DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \
+ default y if DEBUG_LL && (!DEBUG_OMAP2PLUS_UART || !ZBOOT_ROM) && \
(!DEBUG_TEGRA_UART || !ZBOOT_ROM)
help
This option influences the normal decompressor output for
diff --git a/arch/arm/include/debug/omap2plus.S
b/arch/arm/include/debug/omap2plus.S
index 6d867ae..12421f9 100644
--- a/arch/arm/include/debug/omap2plus.S
+++ b/arch/arm/include/debug/omap2plus.S
@@ -58,11 +58,18 @@
#define UART_OFFSET(addr) ((addr) & 0x00ff)
+#if defined(ZIMAGE)
+ omap_uart_phys: .word 0
+ omap_uart_virt: .word 0
+ omap_uart_lsr: .word 0
+#else
+
.pushsection .data
-omap_uart_phys:.word 0
-omap_uart_virt:.word 0
-omap_uart_lsr: .word 0
+ omap_uart_phys: .word 0
+ omap_uart_virt: .word 0
+ omap_uart_lsr: .word 0
.popsection
+#endif
.macro addruart, rp, rv, tmp
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
