Re: [PATCH 0/4] ARM: dts: aspeed: Add Facebook Galaxy100 BMC
On Wed, Nov 11, 2020 at 11:34:10PM +, Joel Stanley wrote: > On Wed, 11 Nov 2020 at 23:23, wrote: > > > > From: Tao Ren > > > > The patch series adds the initial version of device tree for Facebook > > Galaxy100 (AST2400) BMC. > > > > Patch #1 adds common dtsi to minimize duplicated device entries across > > Facebook Network AST2400 BMC device trees. > > > > Patch #2 simplfies Wedge40 device tree by using the common dtsi. > > > > Patch #3 simplfies Wedge100 device tree by using the common dtsi. > > > > Patch #4 adds the initial version of device tree for Facebook Galaxy100 > > BMC. > > Nice. They look good to me. > > Reviewed-by: Joel Stanley > > Is there another person familiar with the design you would like to > review before I merge? Also, Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH 4/5] ARM: dts: aspeed: minipack: Update 64MB FMC flash layout
On Mon, Aug 24, 2020 at 02:19:47PM -0700, rentao.b...@gmail.com wrote: > From: Tao Ren > > Set 64Mb FMC flash layout in Minipack device tree explicitly because the > flash layout was removed from "ast2500-facebook-netbmc-common.dtsi". > > Please note "data0" partition' size is updated to 4MB to be consistent > with other Facebook OpenBMC platforms. > > Signed-off-by: Tao Ren > --- > .../boot/dts/aspeed-bmc-facebook-minipack.dts | 47 ++- > 1 file changed, 45 insertions(+), 2 deletions(-) > Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH 3/5] ARM: dts: aspeed: yamp: Set 32MB FMC flash layout
On Mon, Aug 24, 2020 at 02:19:46PM -0700, rentao.b...@gmail.com wrote: > From: Tao Ren > > Set 32MB FMC flash layout in Yamp device tree explicitly because flash > layout settings were removed from "ast2500-facebook-netbmc-common.dtsi". > > Signed-off-by: Tao Ren > --- > arch/arm/boot/dts/aspeed-bmc-facebook-yamp.dts | 17 + > 1 file changed, 17 insertions(+) > Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH 2/5] ARM: dts: aspeed: cmm: Set 32MB FMC flash layout
On Mon, Aug 24, 2020 at 02:19:45PM -0700, rentao.b...@gmail.com wrote: > From: Tao Ren > > Set 32MB FMC flash layout in CMM device tree explicitly because the flash > layout settings were removed from "ast2500-facebook-netbmc-common.dtsi". > > Signed-off-by: Tao Ren > --- > arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts | 17 + > 1 file changed, 17 insertions(+) > Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH 1/5] ARM: dts: aspeed: Remove flash layout from Facebook AST2500 Common dtsi
On Mon, Aug 24, 2020 at 02:19:44PM -0700, rentao.b...@gmail.com wrote: > From: Tao Ren > > Remove FMC flash layout from ast2500-facebook-netbmc-common.dtsi because > flash size and layout varies across different Facebook AST2500 OpenBMC > platforms. > > Signed-off-by: Tao Ren > --- > .../boot/dts/ast2500-facebook-netbmc-common.dtsi| 13 - > 1 file changed, 13 deletions(-) > Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH v2] hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
Hi Guenter,
Thanks for the initial look at this.
One question for you below...
On Fri, May 29, 2020 at 10:30:16AM -0700, Guenter Roeck wrote:
> On 5/29/20 5:46 AM, Manikandan Elumalai wrote:
> > + /* Enable TEMP1 by default */
> > + config |= ADM1278_TEMP1_EN;
> > + ret = i2c_smbus_write_byte_data(client,
> > + ADM1275_PMON_CONFIG,
> > + config);
> > + if (ret < 0) {
> > + dev_err(&client->dev,
> > + "Failed to enable temperature config\n");
> > + return -ENODEV;
> > + }
>
> This can be handled in a single operation, together with ADM1278_VOUT_EN
> below. There is no need for two separate write operations.
I don't know if you noticed here but the change ends up enabling
TEMP1_EN in all cases. Is this acceptable? If not, do you have any
preference on how it is selected for enablement?
> > /* Enable VOUT if not enabled (it is disabled by default) */
> > if (!(config & ADM1278_VOUT_EN)) {
> > @@ -681,9 +697,6 @@ static int adm1275_probe(struct i2c_client *client,
> > }
> > }
> >
> > - if (config & ADM1278_TEMP1_EN)
> > - info->func[0] |=
> > - PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> > if (config & ADM1278_VIN_EN)
> > info->func[0] |= PMBUS_HAVE_VIN;
> > break;
> >
>
--
Patrick Williams
signature.asc
Description: PGP signature
Re: [PATCH] hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
On Thu, May 28, 2020 at 07:45:23PM +0530, Manikandan Elumalai wrote:
Hi Manikandan,
Adding the PMBus maintainers...
>
> The adm1278 temperature sysfs attribute need it for one of the our openbmc
> platform .
> This functionality is not enabled by default, so PMON_CONFIG needs to be
> modified in order to enable it.
Vijay already mentioned the Signed-off-by here.
Since this is a kernel patch and your first time contributing one, please
read through:
https://www.kernel.org/doc/html/latest/process/1.Intro.html
and the MAINTAINERS file. Another thing you've missed is using the
get_maintainer.pl script to find out who you're suppose to CC. It is
fine to have additional CCs but we're missing the pmbus maintainer on
this patch.
>
> ---
> drivers/hwmon/pmbus/adm1275.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> index 5caa37fb..47b293d 100644
> --- a/drivers/hwmon/pmbus/adm1275.c
> +++ b/drivers/hwmon/pmbus/adm1275.c
> @@ -681,6 +681,21 @@ static int adm1275_probe(struct i2c_client *client,
> }
> }
>
> + config = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG);
> + if (config < 0)
> + return config;
> +
> + /* Enable TEMP1 by defult */
> + config |= ADM1278_TEMP1_EN;
> + ret = i2c_smbus_write_byte_data(client,
> + ADM1275_PMON_CONFIG,
> + config);
> + if (ret < 0) {
> + dev_err(&client->dev,
> + "Failed to enable temperature config\n");
> + return -ENODEV;
> + }
> +
This code might work for your design, but likely doesn't work for
everyone and isn't likely to be accepted in its current state. I think
you need some kind of detection logic here to know if TEMP1_EN *should*
be enabled. Do we need a device-tree entry for this?
> if (config & ADM1278_TEMP1_EN)
> info->func[0] |=
> PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
> --
> 2.7.4
>
--
Patrick Williams
signature.asc
Description: PGP signature
Re: [PATCH v7] ARM: DTS: Aspeed: Add YADRO Nicole BMC
On Wed, Apr 29, 2020 at 02:37:11PM +0300, Alexander Filippov wrote: > Nicole is an OpenPower machine with an Aspeed 2500 BMC SoC manufactured > by YADRO. > > Signed-off-by: Alexander Filippov > --- > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/aspeed-bmc-opp-nicole.dts | 326 > 2 files changed, 327 insertions(+) > create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-nicole.dts > Reviewed-by: Patrick Williams -- Patrick Williams signature.asc Description: PGP signature
Re: [PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs
Thanks for the review Andy.
On Tue, Oct 01, 2019 at 07:29:13PM +0300, Andy Shevchenko wrote:
>
>
> On Tue, Oct 01, 2019 at 10:59:59AM -0500, Patrick Williams wrote:
> There are quite a few people in the Cc list. I'm not sure they all are
> interested in this. I deliberately dropped few names, sorry, if I was
> mistaken.
Agree it was kind of a big list. Just chose what was given to me by
get_maintainer.pl. It seems like there isn't a direct identified
maintainer of this file.
>
> > + if (isr & ISR_RWM) {
> > + u8 byte = 0;
> > +
> > + i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED,
> > + &byte);
> > + writel(byte, _IDBR(i2c));
> > + } else {
> > + i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED,
> > + NULL);
> > + }
>
> Hmm... Perhaps
>
> u8 byte = 0;
>
> i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED, &byte);
> if (isr & ISR_RWM)
> writel(byte, _IDBR(i2c));
>
The two different paths also require READ_REQUEST vs WRITE_REQUESTED. I
could do a ternary there but it seemed more obvious to just unroll the
logic.
--
- Patrick
[PATCH 2/2] i2c: slave-eeprom: support additional models
Add support for emulating the following EEPROMs:
* 24c01 - 1024 bit
* 24c128 - 128k bit
* 24c256 - 256k bit
* 24c512 - 512k bit
The flag bits in the device id were shifted up 1 bit to make
room for saving the 24c512's size. 24c512 uses the full 16-bit
address space of a 2-byte addressable EEPROM.
Signed-off-by: Patrick Williams
---
drivers/i2c/i2c-slave-eeprom.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
index efee56106251..65419441313b 100644
--- a/drivers/i2c/i2c-slave-eeprom.c
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -37,9 +37,9 @@ struct eeprom_data {
u8 buffer[];
};
-#define I2C_SLAVE_BYTELEN GENMASK(15, 0)
-#define I2C_SLAVE_FLAG_ADDR16 BIT(16)
-#define I2C_SLAVE_FLAG_RO BIT(17)
+#define I2C_SLAVE_BYTELEN GENMASK(16, 0)
+#define I2C_SLAVE_FLAG_ADDR16 BIT(17)
+#define I2C_SLAVE_FLAG_RO BIT(18)
#define I2C_SLAVE_DEVICE_MAGIC(_len, _flags) ((_flags) | (_len))
static int i2c_slave_eeprom_slave_cb(struct i2c_client *client,
@@ -171,12 +171,20 @@ static int i2c_slave_eeprom_remove(struct i2c_client
*client)
}
static const struct i2c_device_id i2c_slave_eeprom_id[] = {
+ { "slave-24c01", I2C_SLAVE_DEVICE_MAGIC(1024 / 8, 0) },
+ { "slave-24c01ro", I2C_SLAVE_DEVICE_MAGIC(1024 / 8, I2C_SLAVE_FLAG_RO)
},
{ "slave-24c02", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, 0) },
{ "slave-24c02ro", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, I2C_SLAVE_FLAG_RO)
},
{ "slave-24c32", I2C_SLAVE_DEVICE_MAGIC(32768 / 8,
I2C_SLAVE_FLAG_ADDR16) },
{ "slave-24c32ro", I2C_SLAVE_DEVICE_MAGIC(32768 / 8,
I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
{ "slave-24c64", I2C_SLAVE_DEVICE_MAGIC(65536 / 8,
I2C_SLAVE_FLAG_ADDR16) },
{ "slave-24c64ro", I2C_SLAVE_DEVICE_MAGIC(65536 / 8,
I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c128", I2C_SLAVE_DEVICE_MAGIC(131072 / 8,
I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c128ro", I2C_SLAVE_DEVICE_MAGIC(131072 / 8,
I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c256", I2C_SLAVE_DEVICE_MAGIC(262144 / 8,
I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c256ro", I2C_SLAVE_DEVICE_MAGIC(262144 / 8,
I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c512", I2C_SLAVE_DEVICE_MAGIC(524288 / 8,
I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c512ro", I2C_SLAVE_DEVICE_MAGIC(524288 / 8,
I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
{ }
};
MODULE_DEVICE_TABLE(i2c, i2c_slave_eeprom_id);
--
2.17.2 (Apple Git-113)
[PATCH 1/2] i2c: slave-eeprom: initialize empty eeprom properly
The i2c-slave-eeprom driver emulates at24 class eeprom devices, which come initialized with all 1s. Do the same in the software emulation. Signed-off-by: Patrick Williams --- drivers/i2c/i2c-slave-eeprom.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c index db9763cb4dae..efee56106251 100644 --- a/drivers/i2c/i2c-slave-eeprom.c +++ b/drivers/i2c/i2c-slave-eeprom.c @@ -131,6 +131,8 @@ static int i2c_slave_eeprom_probe(struct i2c_client *client, const struct i2c_de if (!eeprom) return -ENOMEM; + memset(eeprom->buffer, 0xFF, size); + eeprom->idx_write_cnt = 0; eeprom->num_address_bytes = flag_addr16 ? 2 : 1; eeprom->address_mask = size - 1; -- 2.17.2 (Apple Git-113)
[PATCH 2/2] i2c: pxa: remove unused i2c-slave APIs
With the i2c-pxa driver migrated to the standard i2c-slave
APIs, the custom APIs and structures are no longer needed
or used. Remove them.
Signed-off-by: Patrick Williams
---
drivers/i2c/busses/i2c-pxa.c | 1 -
include/linux/i2c-pxa.h | 18 --
include/linux/platform_data/i2c-pxa.h | 4
3 files changed, 23 deletions(-)
delete mode 100644 include/linux/i2c-pxa.h
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index c811646e809f..466e4f681d7a 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -25,7 +25,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/i2c-pxa.h b/include/linux/i2c-pxa.h
deleted file mode 100644
index a897e2b507b6..
--- a/include/linux/i2c-pxa.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _LINUX_I2C_ALGO_PXA_H
-#define _LINUX_I2C_ALGO_PXA_H
-
-typedef enum i2c_slave_event_e {
- I2C_SLAVE_EVENT_START_READ,
- I2C_SLAVE_EVENT_START_WRITE,
- I2C_SLAVE_EVENT_STOP
-} i2c_slave_event_t;
-
-struct i2c_slave_client {
- void *data;
- void (*event)(void *ptr, i2c_slave_event_t event);
- int (*read) (void *ptr);
- void (*write)(void *ptr, unsigned int val);
-};
-
-#endif /* _LINUX_I2C_ALGO_PXA_H */
diff --git a/include/linux/platform_data/i2c-pxa.h
b/include/linux/platform_data/i2c-pxa.h
index cb290092599c..6a9b28399b39 100644
--- a/include/linux/platform_data/i2c-pxa.h
+++ b/include/linux/platform_data/i2c-pxa.h
@@ -55,11 +55,7 @@
*/
#define I2C_ISR_INIT 0x7FF /* status register init */
-struct i2c_slave_client;
-
struct i2c_pxa_platform_data {
- unsigned intslave_addr;
- struct i2c_slave_client *slave;
unsigned intclass;
unsigned intuse_pio :1;
unsigned intfast_mode :1;
--
2.17.2 (Apple Git-113)
[PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs
The i2c subsystem was enhanced circa 2015 to support operating as
an i2c-slave device. Prior to that, the i2c-pxa driver supported
an i2c-slave but had its own APIs. There are no existing in-kernel
drivers or platforms that utilize the i2c-pxa APIs.
Migrate the i2c-pxa driver to the general i2c-slave APIs so that
existing drivers, such as the i2c-slave-eeprom, can be used.
This has been tested with a Marvell EspressoBin, using i2c-pxa and
i2c-slave-eeprom, acting as a slave, and a RaspeberryPi 3, using the
at24 driver, acting as a master.
Signed-off-by: Patrick Williams
---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-pxa.c | 74 +---
2 files changed, 61 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 146ce40d8e0a..d0c79ac9ffdb 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -874,6 +874,7 @@ config I2C_PXA_PCI
config I2C_PXA_SLAVE
bool "Intel PXA2XX I2C Slave comms support"
depends on I2C_PXA && !X86_32
+ select I2C_SLAVE
help
Support I2C slave mode communications on the PXA I2C bus. This
is necessary for systems where the PXA may be a target on the
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 2c3c3d6935c0..c811646e809f 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -180,7 +180,7 @@ struct pxa_i2c {
struct i2c_adapter adap;
struct clk *clk;
#ifdef CONFIG_I2C_PXA_SLAVE
- struct i2c_slave_client *slave;
+ struct i2c_client *slave;
#endif
unsigned intirqlogidx;
@@ -544,22 +544,23 @@ static void i2c_pxa_slave_txempty(struct pxa_i2c *i2c,
u32 isr)
if (isr & ISR_BED) {
/* what should we do here? */
} else {
- int ret = 0;
+ u8 byte = 0;
if (i2c->slave != NULL)
- ret = i2c->slave->read(i2c->slave->data);
+ i2c_slave_event(i2c->slave, I2C_SLAVE_READ_PROCESSED,
+ &byte);
- writel(ret, _IDBR(i2c));
+ writel(byte, _IDBR(i2c));
writel(readl(_ICR(i2c)) | ICR_TB, _ICR(i2c)); /* allow next
byte */
}
}
static void i2c_pxa_slave_rxfull(struct pxa_i2c *i2c, u32 isr)
{
- unsigned int byte = readl(_IDBR(i2c));
+ u8 byte = readl(_IDBR(i2c));
if (i2c->slave != NULL)
- i2c->slave->write(i2c->slave->data, byte);
+ i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_RECEIVED, &byte);
writel(readl(_ICR(i2c)) | ICR_TB, _ICR(i2c));
}
@@ -572,9 +573,18 @@ static void i2c_pxa_slave_start(struct pxa_i2c *i2c, u32
isr)
dev_dbg(&i2c->adap.dev, "SAD, mode is slave-%cx\n",
(isr & ISR_RWM) ? 'r' : 't');
- if (i2c->slave != NULL)
- i2c->slave->event(i2c->slave->data,
-(isr & ISR_RWM) ? I2C_SLAVE_EVENT_START_READ :
I2C_SLAVE_EVENT_START_WRITE);
+ if (i2c->slave != NULL) {
+ if (isr & ISR_RWM) {
+ u8 byte = 0;
+
+ i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED,
+ &byte);
+ writel(byte, _IDBR(i2c));
+ } else {
+ i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED,
+ NULL);
+ }
+ }
/*
* slave could interrupt in the middle of us generating a
@@ -607,7 +617,7 @@ static void i2c_pxa_slave_stop(struct pxa_i2c *i2c)
dev_dbg(&i2c->adap.dev, "ISR: SSD (Slave Stop)\n");
if (i2c->slave != NULL)
- i2c->slave->event(i2c->slave->data, I2C_SLAVE_EVENT_STOP);
+ i2c_slave_event(i2c->slave, I2C_SLAVE_STOP, NULL);
if (i2c_debug > 2)
dev_dbg(&i2c->adap.dev, "ISR: SSD (Slave Stop) acked\n");
@@ -619,6 +629,38 @@ static void i2c_pxa_slave_stop(struct pxa_i2c *i2c)
if (i2c->msg)
i2c_pxa_master_complete(i2c, I2C_RETRY);
}
+
+static int i2c_pxa_slave_reg(struct i2c_client *slave)
+{
+ struct pxa_i2c *i2c = slave->adapter->algo_data;
+
+ if (i2c->slave)
+ return -EBUSY;
+
+ if (!i2c->reg_isar)
+ return -EAFNOSUPPORT;
+
+ i2c->slave = slave;
+ i2c->slave_addr = slave->addr;
+
+ writel(i2c->slave_addr, _ISAR(i2c));
+
+ return 0;
+}
+
+static int i2c_pxa_slave_unreg(struct i2c_client *slave)
+{
+ struct pxa_i2c *i2c = slave->adapter-&g
[PATCH 0/2] i2c: pxa: migrate to i2c-core-slave APIs
i2c-pxa has its own i2c slave APIs rather than using the i2c-core APIs. There are no in-tree drivers currently using this custom slave API set. Migrate the i2c-pxa driver from its own i2c slave APIs to the i2c-core slave APIs so that in-tree slave devices can be used with the i2c-pxa hardware (ex. i2c-slave-eeprom). Patrick Williams (2): i2c: pxa: migrate to new i2c_slave APIs i2c: pxa: remove unused i2c-slave APIs drivers/i2c/busses/Kconfig| 1 + drivers/i2c/busses/i2c-pxa.c | 75 +-- include/linux/i2c-pxa.h | 18 --- include/linux/platform_data/i2c-pxa.h | 4 -- 4 files changed, 61 insertions(+), 37 deletions(-) delete mode 100644 include/linux/i2c-pxa.h -- 2.17.2 (Apple Git-113)
[PATCH] pinctrl: armada-37xx: swap polarity on LED group
The configuration registers for the LED group have inverted
polarity, which puts the GPIO into open-drain state when used in
GPIO mode. Switch to '0' for GPIO and '1' for LED modes.
Fixes: 87466ccd9401 ("pinctrl: armada-37xx: Add pin controller support for
Armada 37xx")
Signed-off-by: Patrick Williams
Cc:
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 6462d3ca7ceb..6310963ce5f0 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -183,10 +183,10 @@ static struct armada_37xx_pin_group
armada_37xx_nb_groups[] = {
PIN_GRP_EXTRA("uart2", 9, 2, BIT(1) | BIT(13) | BIT(14) | BIT(19),
BIT(1) | BIT(13) | BIT(14), BIT(1) | BIT(19),
18, 2, "gpio", "uart"),
- PIN_GRP_GPIO("led0_od", 11, 1, BIT(20), "led"),
- PIN_GRP_GPIO("led1_od", 12, 1, BIT(21), "led"),
- PIN_GRP_GPIO("led2_od", 13, 1, BIT(22), "led"),
- PIN_GRP_GPIO("led3_od", 14, 1, BIT(23), "led"),
+ PIN_GRP_GPIO_2("led0_od", 11, 1, BIT(20), BIT(20), 0, "led"),
+ PIN_GRP_GPIO_2("led1_od", 12, 1, BIT(21), BIT(21), 0, "led"),
+ PIN_GRP_GPIO_2("led2_od", 13, 1, BIT(22), BIT(22), 0, "led"),
+ PIN_GRP_GPIO_2("led3_od", 14, 1, BIT(23), BIT(23), 0, "led"),
};
--
2.17.2 (Apple Git-113)
[PATCH v2] pinctrl: armada-37xx: fix control of pins 32 and up
The 37xx configuration registers are only 32 bits long, so
pins 32-35 spill over into the next register. The calculation
for the register address was done, but the bitmask was not, so
any configuration to pin 32 or above resulted in a bitmask that
overflowed and performed no action.
Fix the register / offset calculation to also adjust the offset.
Fixes: 5715092a458c ("pinctrl: armada-37xx: Add gpio support")
Signed-off-by: Patrick Williams
Acked-by: Gregory CLEMENT
Cc:
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 6462d3ca7ceb..34c1fee52cbe 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -221,11 +221,11 @@ static const struct armada_37xx_pin_data
armada_37xx_pin_sb = {
};
static inline void armada_37xx_update_reg(unsigned int *reg,
- unsigned int offset)
+ unsigned int *offset)
{
/* We never have more than 2 registers */
- if (offset >= GPIO_PER_REG) {
- offset -= GPIO_PER_REG;
+ if (*offset >= GPIO_PER_REG) {
+ *offset -= GPIO_PER_REG;
*reg += sizeof(u32);
}
}
@@ -376,7 +376,7 @@ static inline void armada_37xx_irq_update_reg(unsigned int
*reg,
{
int offset = irqd_to_hwirq(d);
- armada_37xx_update_reg(reg, offset);
+ armada_37xx_update_reg(reg, &offset);
}
static int armada_37xx_gpio_direction_input(struct gpio_chip *chip,
@@ -386,7 +386,7 @@ static int armada_37xx_gpio_direction_input(struct
gpio_chip *chip,
unsigned int reg = OUTPUT_EN;
unsigned int mask;
- armada_37xx_update_reg(®, offset);
+ armada_37xx_update_reg(®, &offset);
mask = BIT(offset);
return regmap_update_bits(info->regmap, reg, mask, 0);
@@ -399,7 +399,7 @@ static int armada_37xx_gpio_get_direction(struct gpio_chip
*chip,
unsigned int reg = OUTPUT_EN;
unsigned int val, mask;
- armada_37xx_update_reg(®, offset);
+ armada_37xx_update_reg(®, &offset);
mask = BIT(offset);
regmap_read(info->regmap, reg, &val);
@@ -413,7 +413,7 @@ static int armada_37xx_gpio_direction_output(struct
gpio_chip *chip,
unsigned int reg = OUTPUT_EN;
unsigned int mask, val, ret;
- armada_37xx_update_reg(®, offset);
+ armada_37xx_update_reg(®, &offset);
mask = BIT(offset);
ret = regmap_update_bits(info->regmap, reg, mask, mask);
@@ -434,7 +434,7 @@ static int armada_37xx_gpio_get(struct gpio_chip *chip,
unsigned int offset)
unsigned int reg = INPUT_VAL;
unsigned int val, mask;
- armada_37xx_update_reg(®, offset);
+ armada_37xx_update_reg(®, &offset);
mask = BIT(offset);
regmap_read(info->regmap, reg, &val);
@@ -449,7 +449,7 @@ static void armada_37xx_gpio_set(struct gpio_chip *chip,
unsigned int offset,
unsigned int reg = OUTPUT_VAL;
unsigned int mask, val;
- armada_37xx_update_reg(®, offset);
+ armada_37xx_update_reg(®, &offset);
mask = BIT(offset);
val = value ? mask : 0;
--
2.17.2 (Apple Git-113)
Re: [PATCH 0/2] pinctl: armada-37xx: fix for pins 32+
On Tue, Jun 25, 2019 at 03:38:59PM +0200, Gregory CLEMENT wrote:
> First you can add my
> Acked-by: Gregory CLEMENT
Thanks for the review Gregory.
> Then as the second patch is a fix, you should add the fix tag: "Fixes:
> 5715092a458c ("pinctrl: armada-37xx: Add gpio support") " as well as the
> 'CC: " tags.
>
> But your change in the first patch made this second patch more difficult
> to backport.
> ...
> Maybe you could change the order of those 2 patches?
Good points. Will do both.
> Actually, when I wrote "_update_reg" I was thinking to the update of the
> variable, whereas with a function named "_calculate_reg" I am expecting
> having the result as a return of the function.
Understand. I can see the ambiguity in both names. How about
"_update_reg_offset"?
> Thanks,
>
> Gregory
>
--
- Patrick
Re: [RFC v1 0/4] ipmi_bmc: framework for IPMI on BMCs
On Mon, Aug 07, 2017 at 08:52:57PM -0700, Brendan Higgins wrote: > Currently, OpenBMC handles all IPMI message routing and handling in userland; > the existing drivers simply provide a file interface for the hardware on the > device. In this patchset, we propose a common file interface to be shared by > all > IPMI hardware interfaces, but also a framework for implementing handlers at > the > kernel level, similar to how the existing OpenIPMI framework supports both > kernel users, as well as misc device file interface. Brendan, Can you expand on why this is a good thing from an OpenBMC perspective? We have a pretty significant set of IPMI providers that run in the userspace daemon(s) and I can't picture more than a very small subset even being possible to run in kernel space without userspace assistance. We also already have an implementation of a RMCP+ daemon that can, and does, share most of its providers with the host-side daemon. -- Patrick Williams signature.asc Description: Digital signature
Re: [PATCH v4] drivers/misc: Add Aspeed LPC control driver
On Fri, Feb 10, 2017 at 03:30:12PM +0100, Greg KH wrote: > On Wed, Feb 08, 2017 at 10:42:47AM +1100, Cyril Bur wrote: > > Signed-off-by: Cyril Bur > > --- > > Without some other reviewed-by: or at least tested-by lines here, I'm > not going to take this. Go poke your fellow ppc people to do some work > here, it shouldn't be up to me to do it for them :( > > thanks, > > greg k-h We are actively using this driver in the OpenBMC project and have exercised it in booting two different server variants (different motherboards entirely). Tested-by: Patrick Williams -- Patrick Williams signature.asc Description: Digital signature
