* [PATCH v3] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Andrew Jeffery @ 2019-05-31 6:32 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190531061207.23079-1-a.filippov@yadro.com>
Hello Alexander,
On Fri, 31 May 2019, at 15:42, Alexander Filippov wrote:
> VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
> by YADRO.
>
> Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
> 2 files changed, 235 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 834cce80d1b8..09a851a4705c 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1261,6 +1261,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-bmc-opp-palmetto.dtb \
> aspeed-bmc-opp-romulus.dtb \
> aspeed-bmc-opp-swift.dtb \
> + aspeed-bmc-opp-vesnin.dtb \
The patch doesn't apply to upstream - the Swift machine only exists in the
OpenBMC kernel tree. Please rebase the patch onto upstream and resend.
> aspeed-bmc-opp-witherspoon.dtb \
> aspeed-bmc-opp-zaius.dtb \
> aspeed-bmc-portwell-neptune.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> new file mode 100644
> index 000000000000..20f07f5bb4f4
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2019 YADRO
> +/dts-v1/;
> +
> +#include "aspeed-g4.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +/ {
> + model = "Vesnin BMC";
> + compatible = "yadro,vesnin-bmc", "aspeed,ast2400";
> +
> + chosen {
> + stdout-path = &uart5;
> + bootargs = "console=ttyS4,115200 earlyprintk";
> + };
> +
> + memory {
> + reg = <0x40000000 0x20000000>;
> + };
> +
> + reserved-memory {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + vga_memory: framebuffer at 5f000000 {
> + no-map;
> + reg = <0x5f000000 0x01000000>; /* 16MB */
> + };
> + flash_memory: region at 5c000000 {
> + no-map;
> + reg = <0x5c000000 0x02000000>; /* 32M */
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + heartbeat {
> + gpios = <&gpio ASPEED_GPIO(R, 4) GPIO_ACTIVE_LOW>;
> + };
> + power_red {
> + gpios = <&gpio ASPEED_GPIO(N, 1) GPIO_ACTIVE_LOW>;
> + };
> +
> + id_blue {
> + gpios = <&gpio ASPEED_GPIO(O, 0) GPIO_ACTIVE_LOW>;
> + };
> +
> + alarm_red {
> + gpios = <&gpio ASPEED_GPIO(N, 6) GPIO_ACTIVE_LOW>;
> + };
> +
> + alarm_yel {
> + gpios = <&gpio ASPEED_GPIO(N, 7) GPIO_ACTIVE_HIGH>;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + button_checkstop {
> + label = "checkstop";
> + linux,code = <74>;
> + gpios = <&gpio ASPEED_GPIO(P, 5) GPIO_ACTIVE_LOW>;
> + };
> +
> + button_identify {
> + label = "identify";
> + linux,code = <152>;
> + gpios = <&gpio ASPEED_GPIO(O, 7) GPIO_ACTIVE_LOW>;
> + };
> + };
> +};
> +
> +&fmc {
> + status = "okay";
> + flash at 0 {
> + status = "okay";
> + m25p,fast-read;
> + label = "bmc";
> +#include "openbmc-flash-layout.dtsi"
> + };
> +};
> +
> +&spi {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_spi1debug_default>;
Is this how the board is strapped? I'm asking in case it's just copy/paste
from Palmetto, which was (unfortunately) strapped this way.
> +
> + flash at 0 {
> + status = "okay";
> + label = "pnor";
> + m25p,fast-read;
> + };
> +};
> +
> +&mac0 {
> + status = "okay";
> +
> + use-ncsi;
> + no-hw-checksum;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_rmii1_default>;
> +};
> +
> +
> +&uart5 {
> + status = "okay";
> +};
> +
> +&lpc_ctrl {
> + status = "okay";
> + memory-region = <&flash_memory>;
> + flash = <&spi>;
> +};
> +
> +&ibt {
> + status = "okay";
> +};
> +
> +&lpc_host {
> + sio_regs: regs {
> + compatible = "aspeed,bmc-misc";
The patches for this are not upstream, and won't make it in their current
form. Please drop this node from the patch.
> + };
> +};
> +
> +&mbox {
> + status = "okay";
This driver is not upstream either, and we plan on dropping it from the
OpenBMC tree too. Please remove this node from the patch.
Cheers,
Andrew
> +};
> +
> +&uart3 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
> +};
> +
> +&i2c0 {
> + status = "okay";
> +
> + eeprom at 50 {
> + compatible = "atmel,24c256";
> + reg = <0x50>;
> + pagesize = <64>;
> + };
> +};
> +
> +&i2c1 {
> + status = "okay";
> +
> + tmp75 at 49 {
> + compatible = "ti,tmp75";
> + reg = <0x49>;
> + };
> +};
> +
> +&i2c2 {
> + status = "okay";
> +};
> +
> +&i2c3 {
> + status = "okay";
> +};
> +
> +&i2c4 {
> + status = "okay";
> +
> + occ-hwmon at 50 {
> + compatible = "ibm,p8-occ-hwmon";
> + reg = <0x50>;
> + };
> +};
> +
> +&i2c5 {
> + status = "okay";
> +
> + occ-hwmon at 51 {
> + compatible = "ibm,p8-occ-hwmon";
> + reg = <0x51>;
> + };
> +};
> +
> +&i2c6 {
> + status = "okay";
> +
> + w83795g at 2f {
> + compatible = "nuvoton,w83795g";
> + reg = <0x2f>;
> + };
> +};
> +
> +&i2c7 {
> + status = "okay";
> +
> + occ-hwmon at 56 {
> + compatible = "ibm,p8-occ-hwmon";
> + reg = <0x56>;
> + };
> +};
> +
> +&i2c9 {
> + status = "okay";
> +};
> +
> +&i2c10 {
> + status = "okay";
> +};
> +
> +&i2c11 {
> + status = "okay";
> +
> + occ-hwmon at 57 {
> + compatible = "ibm,p8-occ-hwmon";
> + reg = <0x57>;
> + };
> +};
> +
> +&i2c12 {
> + status = "okay";
> +
> + rtc at 68 {
> + compatible = "maxim,ds3231";
> + reg = <0x68>;
> + };
> +};
> +
> +&i2c13 {
> + status = "okay";
> +};
> +
> +&vuart {
> + status = "okay";
> +};
> --
> 2.20.1
>
>
^ permalink raw reply
* [PATCH v3] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Alexander Filippov @ 2019-05-31 6:12 UTC (permalink / raw)
To: linux-aspeed
VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
by YADRO.
Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
2 files changed, 235 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 834cce80d1b8..09a851a4705c 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1261,6 +1261,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-opp-palmetto.dtb \
aspeed-bmc-opp-romulus.dtb \
aspeed-bmc-opp-swift.dtb \
+ aspeed-bmc-opp-vesnin.dtb \
aspeed-bmc-opp-witherspoon.dtb \
aspeed-bmc-opp-zaius.dtb \
aspeed-bmc-portwell-neptune.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
new file mode 100644
index 000000000000..20f07f5bb4f4
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2019 YADRO
+/dts-v1/;
+
+#include "aspeed-g4.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+
+/ {
+ model = "Vesnin BMC";
+ compatible = "yadro,vesnin-bmc", "aspeed,ast2400";
+
+ chosen {
+ stdout-path = &uart5;
+ bootargs = "console=ttyS4,115200 earlyprintk";
+ };
+
+ memory {
+ reg = <0x40000000 0x20000000>;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ vga_memory: framebuffer at 5f000000 {
+ no-map;
+ reg = <0x5f000000 0x01000000>; /* 16MB */
+ };
+ flash_memory: region at 5c000000 {
+ no-map;
+ reg = <0x5c000000 0x02000000>; /* 32M */
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ heartbeat {
+ gpios = <&gpio ASPEED_GPIO(R, 4) GPIO_ACTIVE_LOW>;
+ };
+ power_red {
+ gpios = <&gpio ASPEED_GPIO(N, 1) GPIO_ACTIVE_LOW>;
+ };
+
+ id_blue {
+ gpios = <&gpio ASPEED_GPIO(O, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_red {
+ gpios = <&gpio ASPEED_GPIO(N, 6) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_yel {
+ gpios = <&gpio ASPEED_GPIO(N, 7) GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ button_checkstop {
+ label = "checkstop";
+ linux,code = <74>;
+ gpios = <&gpio ASPEED_GPIO(P, 5) GPIO_ACTIVE_LOW>;
+ };
+
+ button_identify {
+ label = "identify";
+ linux,code = <152>;
+ gpios = <&gpio ASPEED_GPIO(O, 7) GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&fmc {
+ status = "okay";
+ flash at 0 {
+ status = "okay";
+ m25p,fast-read;
+ label = "bmc";
+#include "openbmc-flash-layout.dtsi"
+ };
+};
+
+&spi {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1debug_default>;
+
+ flash at 0 {
+ status = "okay";
+ label = "pnor";
+ m25p,fast-read;
+ };
+};
+
+&mac0 {
+ status = "okay";
+
+ use-ncsi;
+ no-hw-checksum;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rmii1_default>;
+};
+
+
+&uart5 {
+ status = "okay";
+};
+
+&lpc_ctrl {
+ status = "okay";
+ memory-region = <&flash_memory>;
+ flash = <&spi>;
+};
+
+&ibt {
+ status = "okay";
+};
+
+&lpc_host {
+ sio_regs: regs {
+ compatible = "aspeed,bmc-misc";
+ };
+};
+
+&mbox {
+ status = "okay";
+};
+
+&uart3 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
+};
+
+&i2c0 {
+ status = "okay";
+
+ eeprom at 50 {
+ compatible = "atmel,24c256";
+ reg = <0x50>;
+ pagesize = <64>;
+ };
+};
+
+&i2c1 {
+ status = "okay";
+
+ tmp75 at 49 {
+ compatible = "ti,tmp75";
+ reg = <0x49>;
+ };
+};
+
+&i2c2 {
+ status = "okay";
+};
+
+&i2c3 {
+ status = "okay";
+};
+
+&i2c4 {
+ status = "okay";
+
+ occ-hwmon at 50 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x50>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+
+ occ-hwmon at 51 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x51>;
+ };
+};
+
+&i2c6 {
+ status = "okay";
+
+ w83795g at 2f {
+ compatible = "nuvoton,w83795g";
+ reg = <0x2f>;
+ };
+};
+
+&i2c7 {
+ status = "okay";
+
+ occ-hwmon at 56 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x56>;
+ };
+};
+
+&i2c9 {
+ status = "okay";
+};
+
+&i2c10 {
+ status = "okay";
+};
+
+&i2c11 {
+ status = "okay";
+
+ occ-hwmon at 57 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x57>;
+ };
+};
+
+&i2c12 {
+ status = "okay";
+
+ rtc at 68 {
+ compatible = "maxim,ds3231";
+ reg = <0x68>;
+ };
+};
+
+&i2c13 {
+ status = "okay";
+};
+
+&vuart {
+ status = "okay";
+};
--
2.20.1
^ permalink raw reply related
* [PATCH v2] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Alexander A. Filippov @ 2019-05-31 6:07 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530203653.GD1561@lunn.ch>
On Thu, May 30, 2019 at 10:36:53PM +0200, Andrew Lunn wrote:
> On Thu, May 30, 2019 at 05:39:33PM +0300, Alexander Filippov wrote:
> > VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
> > by YADRO.
> >
> > Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
> > ---
> > arch/arm/boot/dts/Makefile | 1 +
> > arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
> > 2 files changed, 235 insertions(+)
> > create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 834cce80d1b8..811e9312cf22 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -1259,6 +1259,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> > aspeed-bmc-microsoft-olympus.dtb \
> > aspeed-bmc-opp-lanyang.dtb \
> > aspeed-bmc-opp-palmetto.dtb \
> > + aspeed-bmc-opp-vesnin.dtb \
> > aspeed-bmc-opp-romulus.dtb \
> > aspeed-bmc-opp-swift.dtb \
> > aspeed-bmc-opp-witherspoon.dtb \
>
> Hi Alexander
Hi Andrew,
>
> Still not correctly sorted.
>
Thanks, I've fixed it in V3
> Andrew
^ permalink raw reply
* [PATCH v3 3/8] drivers/soc: xdma: Add user interface
From: Eduardo Valentin @ 2019-05-31 3:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1559153408-31190-4-git-send-email-eajames@linux.ibm.com>
On Wed, May 29, 2019 at 01:10:03PM -0500, Eddie James wrote:
> This commits adds a miscdevice to provide a user interface to the XDMA
> engine. The interface provides the write operation to start DMA
> operations. The DMA parameters are passed as the data to the write call.
> The actual data to transfer is NOT passed through write. Note that both
> directions of DMA operation are accomplished through the write command;
> BMC to host and host to BMC.
>
> The XDMA engine is restricted to only accessing the reserved memory
> space on the AST2500, typically used by the VGA. For this reason, the
> VGA memory space is pooled and allocated with genalloc. Users calling
> mmap allocate pages from this pool for their usage. The space allocated
> by a client will be the space used in the DMA operation. For an
> "upstream" (BMC to host) operation, the data in the client's area will
> be transferred to the host. For a "downstream" (host to BMC) operation,
> the host data will be placed in the client's memory area.
>
> Poll is also provided in order to determine when the DMA operation is
> complete for non-blocking IO.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/soc/aspeed/aspeed-xdma.c | 201 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 201 insertions(+)
>
> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
> index 3dc0ce4..39f6545 100644
> --- a/drivers/soc/aspeed/aspeed-xdma.c
> +++ b/drivers/soc/aspeed/aspeed-xdma.c
> @@ -129,6 +129,8 @@ struct aspeed_xdma {
>
> unsigned long flags;
> unsigned int cmd_idx;
> + struct mutex list_lock;
> + struct mutex start_lock;
> wait_queue_head_t wait;
> struct aspeed_xdma_client *current_client;
>
> @@ -140,6 +142,8 @@ struct aspeed_xdma {
> dma_addr_t cmdq_vga_phys;
> void *cmdq_vga_virt;
> struct gen_pool *vga_pool;
> +
> + struct miscdevice misc;
> };
>
> struct aspeed_xdma_client {
> @@ -331,6 +335,183 @@ static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
> return IRQ_HANDLED;
> }
>
> +static ssize_t aspeed_xdma_write(struct file *file, const char __user *buf,
> + size_t len, loff_t *offset)
> +{
> + int rc;
> + struct aspeed_xdma_op op;
> + struct aspeed_xdma_client *client = file->private_data;
> + struct aspeed_xdma *ctx = client->ctx;
> + u32 offs = client->phys ? (client->phys - ctx->vga_phys) :
> + XDMA_CMDQ_SIZE;
> +
> + if (len != sizeof(struct aspeed_xdma_op))
> + return -EINVAL;
> +
> + rc = copy_from_user(&op, buf, len);
> + if (rc)
> + return rc;
> +
> + if (op.len > (ctx->vga_size - offs) || op.len < XDMA_BYTE_ALIGN)
> + return -EINVAL;
> +
> + if (file->f_flags & O_NONBLOCK) {
> + if (!mutex_trylock(&ctx->start_lock))
> + return -EAGAIN;
> +
> + if (test_bit(XDMA_IN_PRG, &ctx->flags)) {
> + mutex_unlock(&ctx->start_lock);
> + return -EAGAIN;
> + }
> + } else {
> + mutex_lock(&ctx->start_lock);
> +
> + rc = wait_event_interruptible(ctx->wait,
> + !test_bit(XDMA_IN_PRG,
> + &ctx->flags));
> + if (rc) {
> + mutex_unlock(&ctx->start_lock);
> + return -EINTR;
> + }
> + }
> +
> + ctx->current_client = client;
> + set_bit(XDMA_IN_PRG, &client->flags);
> +
> + aspeed_xdma_start(ctx, &op, ctx->vga_phys + offs);
> +
> + mutex_unlock(&ctx->start_lock);
> +
> + if (!(file->f_flags & O_NONBLOCK)) {
> + rc = wait_event_interruptible(ctx->wait,
> + !test_bit(XDMA_IN_PRG,
> + &ctx->flags));
> + if (rc)
> + return -EINTR;
> + }
> +
> + return len;
> +}
> +
> +static __poll_t aspeed_xdma_poll(struct file *file,
> + struct poll_table_struct *wait)
> +{
> + __poll_t mask = 0;
> + __poll_t req = poll_requested_events(wait);
> + struct aspeed_xdma_client *client = file->private_data;
> + struct aspeed_xdma *ctx = client->ctx;
> +
> + if (req & (EPOLLIN | EPOLLRDNORM)) {
> + if (test_bit(XDMA_IN_PRG, &client->flags))
> + poll_wait(file, &ctx->wait, wait);
> +
> + if (!test_bit(XDMA_IN_PRG, &client->flags))
> + mask |= EPOLLIN | EPOLLRDNORM;
> + }
> +
> + if (req & (EPOLLOUT | EPOLLWRNORM)) {
> + if (test_bit(XDMA_IN_PRG, &ctx->flags))
> + poll_wait(file, &ctx->wait, wait);
> +
> + if (!test_bit(XDMA_IN_PRG, &ctx->flags))
> + mask |= EPOLLOUT | EPOLLWRNORM;
> + }
> +
> + return mask;
> +}
> +
> +static void aspeed_xdma_vma_close(struct vm_area_struct *vma)
> +{
> + struct aspeed_xdma_client *client = vma->vm_private_data;
> +
> + gen_pool_free(client->ctx->vga_pool, (unsigned long)client->virt,
> + client->size);
> +
> + client->virt = NULL;
> + client->phys = 0;
> + client->size = 0;
> +}
> +
> +static const struct vm_operations_struct aspeed_xdma_vm_ops = {
> + .close = aspeed_xdma_vma_close,
> +};
> +
> +static int aspeed_xdma_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + int rc;
> + struct aspeed_xdma_client *client = file->private_data;
> + struct aspeed_xdma *ctx = client->ctx;
> +
> + /* restrict file to one mapping */
> + if (client->size)
> + return -ENOMEM;
> +
> + client->size = vma->vm_end - vma->vm_start;
> + client->virt = gen_pool_dma_alloc(ctx->vga_pool, client->size,
> + &client->phys);
> + if (!client->virt) {
> + client->phys = 0;
> + client->size = 0;
> + return -ENOMEM;
> + }
> +
> + vma->vm_pgoff = (client->phys - ctx->vga_phys) >> PAGE_SHIFT;
> + vma->vm_ops = &aspeed_xdma_vm_ops;
> + vma->vm_private_data = client;
> +
> + rc = dma_mmap_coherent(ctx->dev, vma, ctx->vga_virt, ctx->vga_dma,
> + ctx->vga_size);
> + if (rc) {
> + gen_pool_free(ctx->vga_pool, (unsigned long)client->virt,
> + client->size);
> +
> + client->virt = NULL;
> + client->phys = 0;
> + client->size = 0;
> + return rc;
> + }
> +
> + dev_dbg(ctx->dev, "mmap: v[%08lx] to p[%08x], s[%08x]\n",
> + vma->vm_start, (u32)client->phys, client->size);
> +
> + return 0;
> +}
> +
> +static int aspeed_xdma_open(struct inode *inode, struct file *file)
> +{
> + struct miscdevice *misc = file->private_data;
> + struct aspeed_xdma *ctx = container_of(misc, struct aspeed_xdma, misc);
> + struct aspeed_xdma_client *client = kzalloc(sizeof(*client),
> + GFP_KERNEL);
> +
> + if (!client)
> + return -ENOMEM;
> +
> + client->ctx = ctx;
> + file->private_data = client;
> + return 0;
> +}
> +
> +static int aspeed_xdma_release(struct inode *inode, struct file *file)
> +{
> + struct aspeed_xdma_client *client = file->private_data;
> +
> + if (client->ctx->current_client == client)
> + client->ctx->current_client = NULL;
> +
> + kfree(client);
> + return 0;
> +}
> +
> +static const struct file_operations aspeed_xdma_fops = {
> + .owner = THIS_MODULE,
> + .write = aspeed_xdma_write,
> + .poll = aspeed_xdma_poll,
> + .mmap = aspeed_xdma_mmap,
> + .open = aspeed_xdma_open,
> + .release = aspeed_xdma_release,
> +};
> +
> static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
> {
> int rc;
> @@ -431,6 +612,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
> ctx->dev = dev;
> platform_set_drvdata(pdev, ctx);
> init_waitqueue_head(&ctx->wait);
> + mutex_init(&ctx->list_lock);
> + mutex_init(&ctx->start_lock);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> ctx->base = devm_ioremap_resource(dev, res);
> @@ -482,6 +665,23 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
>
> aspeed_xdma_init_eng(ctx);
>
> + ctx->misc.minor = MISC_DYNAMIC_MINOR;
> + ctx->misc.fops = &aspeed_xdma_fops;
> + ctx->misc.name = "xdma";
tiny bit here, should this be named more specific ? something like "aspeed_xdma" ?
> + ctx->misc.parent = dev;
> + rc = misc_register(&ctx->misc);
> + if (rc) {
> + dev_err(dev, "Unable to register xdma miscdevice.\n");
> +
> + gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
> + XDMA_CMDQ_SIZE);
> + dma_free_coherent(dev, ctx->vga_size, ctx->vga_virt,
> + ctx->vga_dma);
> + dma_release_declared_memory(dev);
> + reset_control_assert(ctx->reset);
> + return rc;
> + }
> +
> return 0;
> }
>
> @@ -489,6 +689,7 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
> {
> struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
>
> + misc_deregister(&ctx->misc);
> gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
> XDMA_CMDQ_SIZE);
> dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
> --
> 1.8.3.1
>
--
All the best,
Eduardo Valentin
^ permalink raw reply
* [PATCH v3 5/8] drivers/soc: xdma: Add PCI device configuration sysfs
From: Eduardo Valentin @ 2019-05-31 3:45 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1559153408-31190-6-git-send-email-eajames@linux.ibm.com>
On Wed, May 29, 2019 at 01:10:05PM -0500, Eddie James wrote:
> The AST2500 has two PCI devices embedded. The XDMA engine can use either
> device to perform DMA transfers. Users need the capability to choose
> which device to use. This commit therefore adds two sysfs files that
> toggle the AST2500 and XDMA engine between the two PCI devices.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/soc/aspeed/aspeed-xdma.c | 103 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
> index 39f6545..ddd5e1e 100644
> --- a/drivers/soc/aspeed/aspeed-xdma.c
> +++ b/drivers/soc/aspeed/aspeed-xdma.c
> @@ -143,6 +143,7 @@ struct aspeed_xdma {
> void *cmdq_vga_virt;
> struct gen_pool *vga_pool;
>
> + char pcidev[4];
> struct miscdevice misc;
> };
>
> @@ -165,6 +166,10 @@ struct aspeed_xdma_client {
> SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
> SCU_PCIE_CONF_RSVD;
>
> +static char *_pcidev = "vga";
> +module_param_named(pcidev, _pcidev, charp, 0600);
> +MODULE_PARM_DESC(pcidev, "Default PCI device used by XDMA engine for DMA ops");
> +
> static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
> {
> u32 v = 0;
> @@ -512,7 +517,7 @@ static int aspeed_xdma_release(struct inode *inode, struct file *file)
> .release = aspeed_xdma_release,
> };
>
> -static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
> +static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx, u32 conf)
> {
> int rc;
> u32 scu_conf = 0;
> @@ -522,7 +527,7 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
> const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
> void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
>
> - aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
> + aspeed_scu_pcie_write(ctx, conf);
>
> regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
> ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
> @@ -598,10 +603,91 @@ static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
> return rc;
> }
>
> +static int aspeed_xdma_change_pcie_conf(struct aspeed_xdma *ctx, u32 conf)
> +{
> + int rc;
> +
> + mutex_lock(&ctx->start_lock);
> + rc = wait_event_interruptible_timeout(ctx->wait,
> + !test_bit(XDMA_IN_PRG,
> + &ctx->flags),
> + msecs_to_jiffies(1000));
> + if (rc < 0) {
> + mutex_unlock(&ctx->start_lock);
> + return -EINTR;
> + }
> +
> + /* previous op didn't complete, wake up waiters anyway */
> + if (!rc)
> + wake_up_interruptible_all(&ctx->wait);
> +
> + reset_control_assert(ctx->reset);
> + msleep(10);
> +
> + aspeed_scu_pcie_write(ctx, conf);
> + msleep(10);
> +
> + reset_control_deassert(ctx->reset);
> + msleep(10);
> +
> + aspeed_xdma_init_eng(ctx);
> +
> + mutex_unlock(&ctx->start_lock);
> +
> + return 0;
> +}
> +
> +static int aspeed_xdma_pcidev_to_conf(struct aspeed_xdma *ctx,
> + const char *pcidev, u32 *conf)
> +{
> + if (!strcasecmp(pcidev, "vga")) {
> + *conf = aspeed_xdma_vga_pcie_conf;
> + return 0;
> + }
> +
> + if (!strcasecmp(pcidev, "bmc")) {
> + *conf = aspeed_xdma_bmc_pcie_conf;
> + return 0;
> + }
strncasecmp()?
> +
> + return -EINVAL;
> +}
> +
> +static ssize_t aspeed_xdma_show_pcidev(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct aspeed_xdma *ctx = dev_get_drvdata(dev);
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%s", ctx->pcidev);
> +}
> +
> +static ssize_t aspeed_xdma_store_pcidev(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 conf;
> + struct aspeed_xdma *ctx = dev_get_drvdata(dev);
> + int rc = aspeed_xdma_pcidev_to_conf(ctx, buf, &conf);
> +
> + if (rc)
> + return rc;
> +
> + rc = aspeed_xdma_change_pcie_conf(ctx, conf);
> + if (rc)
> + return rc;
> +
> + strcpy(ctx->pcidev, buf);
should we use strncpy() instead?
> + return count;
> +}
> +static DEVICE_ATTR(pcidev, 0644, aspeed_xdma_show_pcidev,
> + aspeed_xdma_store_pcidev);
> +
> static int aspeed_xdma_probe(struct platform_device *pdev)
> {
> int irq;
> int rc;
> + u32 conf;
> struct resource *res;
> struct device *dev = &pdev->dev;
> struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> @@ -657,7 +743,14 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
>
> msleep(10);
>
> - rc = aspeed_xdma_init_mem(ctx);
> + if (aspeed_xdma_pcidev_to_conf(ctx, _pcidev, &conf)) {
> + conf = aspeed_xdma_vga_pcie_conf;
> + strcpy(ctx->pcidev, "vga");
> + } else {
> + strcpy(ctx->pcidev, _pcidev);
> + }
same...
> +
> + rc = aspeed_xdma_init_mem(ctx, conf);
> if (rc) {
> reset_control_assert(ctx->reset);
> return rc;
> @@ -682,6 +775,8 @@ static int aspeed_xdma_probe(struct platform_device *pdev)
> return rc;
> }
>
> + device_create_file(dev, &dev_attr_pcidev);
Should we consider using one of the default attributes here instead of device_create_file()?
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
BTW, was this ABI documented? Is this the same file documented in patch 2?
> +
> return 0;
> }
>
> @@ -689,6 +784,8 @@ static int aspeed_xdma_remove(struct platform_device *pdev)
> {
> struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
>
> + device_remove_file(ctx->dev, &dev_attr_pcidev);
> +
> misc_deregister(&ctx->misc);
> gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
> XDMA_CMDQ_SIZE);
> --
> 1.8.3.1
>
--
All the best,
Eduardo Valentin
^ permalink raw reply
* [PATCH v3 2/8] drivers/soc: Add Aspeed XDMA Engine Driver
From: Eduardo Valentin @ 2019-05-31 3:31 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1559153408-31190-3-git-send-email-eajames@linux.ibm.com>
On Wed, May 29, 2019 at 01:10:02PM -0500, Eddie James wrote:
> The XDMA engine embedded in the AST2500 SOC performs PCI DMA operations
> between the SOC (acting as a BMC) and a host processor in a server.
>
> This commit adds a driver to control the XDMA engine and adds functions
> to initialize the hardware and memory and start DMA operations.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> MAINTAINERS | 10 +
> drivers/soc/aspeed/Kconfig | 8 +
> drivers/soc/aspeed/Makefile | 1 +
> drivers/soc/aspeed/aspeed-xdma.c | 520 +++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/aspeed-xdma.h | 26 ++
> 5 files changed, 565 insertions(+)
> create mode 100644 drivers/soc/aspeed/aspeed-xdma.c
> create mode 100644 include/uapi/linux/aspeed-xdma.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7e09dda..84e2b62 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2584,6 +2584,16 @@ S: Maintained
> F: drivers/media/platform/aspeed-video.c
> F: Documentation/devicetree/bindings/media/aspeed-video.txt
>
> +ASPEED XDMA ENGINE DRIVER
> +M: Eddie James <eajames@linux.ibm.com>
> +L: linux-aspeed at lists.ozlabs.org (moderated for non-subscribers)
> +L: linux-kernel at vger.kernel.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/misc/aspeed,xdma.txt
> +F: Documentation/ABI/testing/sysfs-devices-platform-aspeed-xdma
> +F: drivers/soc/aspeed/aspeed-xdma.c
> +F: include/uapi/linux/aspeed-xdma.h
> +
> ASUS NOTEBOOKS AND EEEPC ACPI/WMI EXTRAS DRIVERS
> M: Corentin Chary <corentin.chary@gmail.com>
> L: acpi4asus-user at lists.sourceforge.net
> diff --git a/drivers/soc/aspeed/Kconfig b/drivers/soc/aspeed/Kconfig
> index 323e177..8b08310 100644
> --- a/drivers/soc/aspeed/Kconfig
> +++ b/drivers/soc/aspeed/Kconfig
> @@ -29,4 +29,12 @@ config ASPEED_P2A_CTRL
> ioctl()s, the driver also provides an interface for userspace mappings to
> a pre-defined region.
>
> +config ASPEED_XDMA
> + tristate "Aspeed XDMA Engine Driver"
> + depends on SOC_ASPEED && REGMAP && MFD_SYSCON && HAS_DMA
> + help
> + Enable support for the Aspeed XDMA Engine found on the Aspeed AST2500
> + SOC. The XDMA engine can perform automatic PCI DMA operations between
> + the AST2500 (acting as a BMC) and a host processor.
> +
> endmenu
> diff --git a/drivers/soc/aspeed/Makefile b/drivers/soc/aspeed/Makefile
> index b64be47..977b046 100644
> --- a/drivers/soc/aspeed/Makefile
> +++ b/drivers/soc/aspeed/Makefile
> @@ -2,3 +2,4 @@
> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> obj-$(CONFIG_ASPEED_LPC_SNOOP) += aspeed-lpc-snoop.o
> obj-$(CONFIG_ASPEED_P2A_CTRL) += aspeed-p2a-ctrl.o
> +obj-$(CONFIG_ASPEED_XDMA) += aspeed-xdma.o
> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
> new file mode 100644
> index 0000000..3dc0ce4
> --- /dev/null
> +++ b/drivers/soc/aspeed/aspeed-xdma.c
> @@ -0,0 +1,520 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright IBM Corp 2019
> +
> +#include <linux/aspeed-xdma.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/fs.h>
> +#include <linux/genalloc.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/list.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/poll.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/string.h>
> +#include <linux/uaccess.h>
> +#include <linux/wait.h>
> +
> +#define DEVICE_NAME "aspeed-xdma"
> +
> +#define SCU_STRAP 0x070
> +#define SCU_STRAP_VGA_MEM GENMASK(3, 2)
> +
> +#define SCU_PCIE_CONF 0x180
> +#define SCU_PCIE_CONF_VGA_EN BIT(0)
> +#define SCU_PCIE_CONF_VGA_EN_MMIO BIT(1)
> +#define SCU_PCIE_CONF_VGA_EN_LPC BIT(2)
> +#define SCU_PCIE_CONF_VGA_EN_MSI BIT(3)
> +#define SCU_PCIE_CONF_VGA_EN_MCTP BIT(4)
> +#define SCU_PCIE_CONF_VGA_EN_IRQ BIT(5)
> +#define SCU_PCIE_CONF_VGA_EN_DMA BIT(6)
> +#define SCU_PCIE_CONF_BMC_EN BIT(8)
> +#define SCU_PCIE_CONF_BMC_EN_MMIO BIT(9)
> +#define SCU_PCIE_CONF_BMC_EN_MSI BIT(11)
> +#define SCU_PCIE_CONF_BMC_EN_MCTP BIT(12)
> +#define SCU_PCIE_CONF_BMC_EN_IRQ BIT(13)
> +#define SCU_PCIE_CONF_BMC_EN_DMA BIT(14)
> +#define SCU_PCIE_CONF_RSVD GENMASK(19, 18)
> +
> +#define SDMC_CONF 0x004
> +#define SDMC_CONF_MEM GENMASK(1, 0)
> +#define SDMC_REMAP 0x008
> +#define SDMC_REMAP_MAGIC GENMASK(17, 16)
> +
> +#define XDMA_CMD_SIZE 4
> +#define XDMA_CMDQ_SIZE PAGE_SIZE
> +#define XDMA_BYTE_ALIGN 16
> +#define XDMA_MAX_LINE_SIZE BIT(10)
> +#define XDMA_NUM_CMDS \
> + (XDMA_CMDQ_SIZE / sizeof(struct aspeed_xdma_cmd))
> +#define XDMA_NUM_DEBUGFS_REGS 6
> +
> +#define XDMA_CMD_BMC_CHECK BIT(0)
> +#define XDMA_CMD_BMC_ADDR GENMASK(29, 4)
> +#define XDMA_CMD_BMC_DIR_US BIT(31)
> +
> +#define XDMA_CMD_COMM1_HI_HOST_PITCH GENMASK(14, 3)
> +#define XDMA_CMD_COMM1_HI_BMC_PITCH GENMASK(30, 19)
> +
> +#define XDMA_CMD_CONF_CHECK BIT(1)
> +#define XDMA_CMD_CONF_LINE_SIZE GENMASK(14, 4)
> +#define XDMA_CMD_CONF_IRQ_BMC BIT(15)
> +#define XDMA_CMD_CONF_NUM_LINES GENMASK(27, 16)
> +#define XDMA_CMD_CONF_IRQ BIT(31)
> +
> +#define XDMA_CMD_ID_UPDIR GENMASK(17, 16)
> +#define XDMA_CMD_ID_UPDIR_BMC 0
> +#define XDMA_CMD_ID_UPDIR_HOST 1
> +#define XDMA_CMD_ID_UPDIR_VGA 2
> +
> +#define XDMA_DS_PCIE_REQ_SIZE_128 0
> +#define XDMA_DS_PCIE_REQ_SIZE_256 1
> +#define XDMA_DS_PCIE_REQ_SIZE_512 2
> +#define XDMA_DS_PCIE_REQ_SIZE_1K 3
> +#define XDMA_DS_PCIE_REQ_SIZE_2K 4
> +#define XDMA_DS_PCIE_REQ_SIZE_4K 5
> +
> +#define XDMA_BMC_CMD_QUEUE_ADDR 0x10
> +#define XDMA_BMC_CMD_QUEUE_ENDP 0x14
> +#define XDMA_BMC_CMD_QUEUE_WRITEP 0x18
> +#define XDMA_BMC_CMD_QUEUE_READP 0x1c
> +#define XDMA_BMC_CMD_QUEUE_READP_MAGIC 0xee882266
> +#define XDMA_CTRL 0x20
> +#define XDMA_CTRL_US_COMP BIT(4)
> +#define XDMA_CTRL_DS_COMP BIT(5)
> +#define XDMA_CTRL_DS_DIRTY BIT(6)
> +#define XDMA_CTRL_DS_PCIE_REQ_SIZE GENMASK(19, 17)
> +#define XDMA_CTRL_DS_DATA_TIMEOUT BIT(28)
> +#define XDMA_CTRL_DS_CHECK_ID BIT(29)
> +#define XDMA_STATUS 0x24
> +#define XDMA_STATUS_US_COMP BIT(4)
> +#define XDMA_STATUS_DS_COMP BIT(5)
> +
> +enum {
> + XDMA_IN_PRG,
> + XDMA_UPSTREAM,
> +};
> +
> +struct aspeed_xdma_cmd {
> + u32 host_addr_lo;
> + u32 host_addr_hi;
> + u32 bmc_addr;
> + u32 comm1_hi;
> + u32 conf;
> + u32 id;
> + u32 resv0;
> + u32 resv1;
> +};
> +
> +struct aspeed_xdma_client;
> +
> +struct aspeed_xdma {
> + struct device *dev;
> + void __iomem *base;
> + struct regmap *scu;
> + struct reset_control *reset;
> +
> + unsigned long flags;
interesting.. why do we need a long size flags field when we just toggle two bits?
From a quick glance, looks like we use this to check for XDMA_IN_PRG and XDMA_UPSTREAM only..
> + unsigned int cmd_idx;
> + wait_queue_head_t wait;
> + struct aspeed_xdma_client *current_client;
> +
> + u32 vga_phys;
> + u32 vga_size;
> + dma_addr_t vga_dma;
> + void *cmdq;
> + void *vga_virt;
> + dma_addr_t cmdq_vga_phys;
> + void *cmdq_vga_virt;
> + struct gen_pool *vga_pool;
> +};
> +
> +struct aspeed_xdma_client {
> + struct aspeed_xdma *ctx;
> +
> + unsigned long flags;
same
> + void *virt;
> + dma_addr_t phys;
> + u32 size;
> +};
> +
> +static const u32 aspeed_xdma_bmc_pcie_conf = SCU_PCIE_CONF_BMC_EN |
> + SCU_PCIE_CONF_BMC_EN_MSI | SCU_PCIE_CONF_BMC_EN_MCTP |
> + SCU_PCIE_CONF_BMC_EN_IRQ | SCU_PCIE_CONF_BMC_EN_DMA |
> + SCU_PCIE_CONF_RSVD;
> +
> +static const u32 aspeed_xdma_vga_pcie_conf = SCU_PCIE_CONF_VGA_EN |
> + SCU_PCIE_CONF_VGA_EN_MSI | SCU_PCIE_CONF_VGA_EN_MCTP |
> + SCU_PCIE_CONF_VGA_EN_IRQ | SCU_PCIE_CONF_VGA_EN_DMA |
> + SCU_PCIE_CONF_RSVD;
> +
> +static void aspeed_scu_pcie_write(struct aspeed_xdma *ctx, u32 conf)
> +{
> + u32 v = 0;
> +
> + regmap_write(ctx->scu, SCU_PCIE_CONF, conf);
> + regmap_read(ctx->scu, SCU_PCIE_CONF, &v);
> +
> + dev_dbg(ctx->dev, "write scu pcie_conf[%08x]\n", v);
> +}
> +
> +static u32 aspeed_xdma_reg_read(struct aspeed_xdma *ctx, u32 reg)
> +{
> + u32 v = readl(ctx->base + reg);
> +
> + dev_dbg(ctx->dev, "read %02x[%08x]\n", reg, v);
> + return v;
> +}
> +
> +static void aspeed_xdma_reg_write(struct aspeed_xdma *ctx, u32 reg, u32 val)
> +{
> + writel(val, ctx->base + reg);
> + dev_dbg(ctx->dev, "write %02x[%08x]\n", reg, readl(ctx->base + reg));
> +}
> +
> +static void aspeed_xdma_init_eng(struct aspeed_xdma *ctx)
> +{
> + const u32 ctrl = XDMA_CTRL_US_COMP | XDMA_CTRL_DS_COMP |
> + XDMA_CTRL_DS_DIRTY | FIELD_PREP(XDMA_CTRL_DS_PCIE_REQ_SIZE,
> + XDMA_DS_PCIE_REQ_SIZE_256) |
> + XDMA_CTRL_DS_DATA_TIMEOUT | XDMA_CTRL_DS_CHECK_ID;
> +
> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ENDP,
> + XDMA_CMD_SIZE * XDMA_NUM_CMDS);
> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_READP,
> + XDMA_BMC_CMD_QUEUE_READP_MAGIC);
> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP, 0);
> + aspeed_xdma_reg_write(ctx, XDMA_CTRL, ctrl);
> +
> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_ADDR,
> + ctx->cmdq_vga_phys);
> +
> + ctx->cmd_idx = 0;
> + ctx->flags = 0;
> +}
> +
> +static void aspeed_xdma_reset(struct aspeed_xdma *ctx)
> +{
> + reset_control_assert(ctx->reset);
> +
> + msleep(10);
> +
> + reset_control_deassert(ctx->reset);
> +
> + msleep(10);
Why 10ms?
> +
> + aspeed_xdma_init_eng(ctx);
> +}
> +
> +static void aspeed_xdma_start(struct aspeed_xdma *ctx,
> + struct aspeed_xdma_op *op, u32 bmc_addr)
> +{
> + u32 conf = XDMA_CMD_CONF_CHECK | XDMA_CMD_CONF_IRQ_BMC |
> + XDMA_CMD_CONF_IRQ;
> + unsigned int line_size = op->len / XDMA_BYTE_ALIGN;
> + unsigned int num_lines = 1;
> + unsigned int nidx = (ctx->cmd_idx + 1) % XDMA_NUM_CMDS;
> + unsigned int pitch = 1;
> + struct aspeed_xdma_cmd *cmd =
> + &(((struct aspeed_xdma_cmd *)ctx->cmdq)[ctx->cmd_idx]);
> +
> + if (line_size > XDMA_MAX_LINE_SIZE) {
> + unsigned int rem;
> + unsigned int total;
> +
> + num_lines = line_size / XDMA_MAX_LINE_SIZE;
> + total = XDMA_MAX_LINE_SIZE * num_lines;
> + rem = line_size - total;
> + line_size = XDMA_MAX_LINE_SIZE;
> + pitch = line_size;
> +
> + if (rem) {
> + unsigned int offs = total * XDMA_BYTE_ALIGN;
> + u32 r_bmc_addr = bmc_addr + offs;
> + u64 r_host_addr = op->host_addr + (u64)offs;
> + struct aspeed_xdma_cmd *r_cmd =
> + &(((struct aspeed_xdma_cmd *)ctx->cmdq)[nidx]);
> +
> + r_cmd->host_addr_lo =
> + (u32)(r_host_addr & 0xFFFFFFFFULL);
> + r_cmd->host_addr_hi = (u32)(r_host_addr >> 32ULL);
> + r_cmd->bmc_addr = (r_bmc_addr & XDMA_CMD_BMC_ADDR) |
> + XDMA_CMD_BMC_CHECK |
> + (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
> + r_cmd->conf = conf |
> + FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, rem) |
> + FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, 1);
> + r_cmd->comm1_hi =
> + FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, 1) |
> + FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, 1);
> +
> + /* do not trigger IRQ for first command */
> + conf = XDMA_CMD_CONF_CHECK;
> +
> + nidx = (nidx + 1) % XDMA_NUM_CMDS;
> + }
> +
> + /* undocumented formula to get required number of lines */
> + num_lines = (num_lines * 2) - 1;
> + }
> +
> + /* ctrl == 0 indicates engine hasn't started properly; restart it */
> + if (!aspeed_xdma_reg_read(ctx, XDMA_CTRL))
> + aspeed_xdma_reset(ctx);
> +
> + cmd->host_addr_lo = (u32)(op->host_addr & 0xFFFFFFFFULL);
> + cmd->host_addr_hi = (u32)(op->host_addr >> 32ULL);
> + cmd->bmc_addr = (bmc_addr & XDMA_CMD_BMC_ADDR) | XDMA_CMD_BMC_CHECK |
> + (op->upstream ? XDMA_CMD_BMC_DIR_US : 0);
> + cmd->conf = conf |
> + FIELD_PREP(XDMA_CMD_CONF_LINE_SIZE, line_size) |
> + FIELD_PREP(XDMA_CMD_CONF_NUM_LINES, num_lines);
> + cmd->comm1_hi = FIELD_PREP(XDMA_CMD_COMM1_HI_HOST_PITCH, pitch) |
> + FIELD_PREP(XDMA_CMD_COMM1_HI_BMC_PITCH, pitch);
> +
> + memcpy(ctx->cmdq_vga_virt, ctx->cmdq, XDMA_CMDQ_SIZE);
> +
> + if (op->upstream)
> + set_bit(XDMA_UPSTREAM, &ctx->flags);
> + else
> + clear_bit(XDMA_UPSTREAM, &ctx->flags);
> +
> + set_bit(XDMA_IN_PRG, &ctx->flags);
> +
> + aspeed_xdma_reg_write(ctx, XDMA_BMC_CMD_QUEUE_WRITEP,
> + nidx * XDMA_CMD_SIZE);
> + ctx->cmd_idx = nidx;
> +}
> +
> +static void aspeed_xdma_done(struct aspeed_xdma *ctx)
> +{
> + if (ctx->current_client) {
> + clear_bit(XDMA_IN_PRG, &ctx->current_client->flags);
> +
> + ctx->current_client = NULL;
> + }
> +
> + clear_bit(XDMA_IN_PRG, &ctx->flags);
> + wake_up_interruptible_all(&ctx->wait);
> +}
> +
> +static irqreturn_t aspeed_xdma_irq(int irq, void *arg)
> +{
> + struct aspeed_xdma *ctx = arg;
> + u32 status = aspeed_xdma_reg_read(ctx, XDMA_STATUS);
> +
> + if (status & XDMA_STATUS_US_COMP) {
> + if (test_bit(XDMA_UPSTREAM, &ctx->flags))
> + aspeed_xdma_done(ctx);
> + }
> +
> + if (status & XDMA_STATUS_DS_COMP) {
> + if (!test_bit(XDMA_UPSTREAM, &ctx->flags))
> + aspeed_xdma_done(ctx);
> + }
> +
> + aspeed_xdma_reg_write(ctx, XDMA_STATUS, status);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int aspeed_xdma_init_mem(struct aspeed_xdma *ctx)
> +{
> + int rc;
> + u32 scu_conf = 0;
> + u32 mem_size = 0x20000000;
> + const u32 mem_sizes[4] = { 0x8000000, 0x10000000, 0x20000000,
> + 0x40000000 };
> + const u32 vga_sizes[4] = { 0x800000, 0x1000000, 0x2000000, 0x4000000 };
> + void __iomem *sdmc_base = ioremap(0x1e6e0000, 0x100);
> +
Should these come from fw specification? Say device tree mem reserved nodes?
> + aspeed_scu_pcie_write(ctx, aspeed_xdma_vga_pcie_conf);
> +
> + regmap_read(ctx->scu, SCU_STRAP, &scu_conf);
> + ctx->vga_size = vga_sizes[FIELD_GET(SCU_STRAP_VGA_MEM, scu_conf)];
> +
> + if (sdmc_base) {
> + u32 sdmc = readl(sdmc_base + SDMC_CONF);
> + u32 remap = readl(sdmc_base + SDMC_REMAP);
> +
> + remap |= SDMC_REMAP_MAGIC;
> + writel(remap, sdmc_base + SDMC_REMAP);
> + remap = readl(sdmc_base + SDMC_REMAP);
> +
> + mem_size = mem_sizes[sdmc & SDMC_CONF_MEM];
> + iounmap(sdmc_base);
> + }
> +
> + ctx->vga_phys = (mem_size - ctx->vga_size) + 0x80000000;
> +
> + ctx->cmdq = devm_kzalloc(ctx->dev, XDMA_CMDQ_SIZE, GFP_KERNEL);
> + if (!ctx->cmdq) {
> + dev_err(ctx->dev, "Failed to allocate command queue.\n");
> + return -ENOMEM;
> + }
> +
> + rc = dma_set_mask_and_coherent(ctx->dev, DMA_BIT_MASK(32));
> + if (rc) {
> + dev_err(ctx->dev, "Failed to set DMA mask: %d.\n", rc);
> + return rc;
> + }
> +
> + rc = dma_declare_coherent_memory(ctx->dev, ctx->vga_phys,
> + ctx->vga_phys, ctx->vga_size);
> + if (rc) {
> + dev_err(ctx->dev, "Failed to declare coherent memory: %d.\n",
> + rc);
> + return rc;
> + }
> +
> + ctx->vga_virt = dma_alloc_coherent(ctx->dev, ctx->vga_size,
> + &ctx->vga_dma, GFP_KERNEL);
> + if (!ctx->vga_virt) {
> + dev_err(ctx->dev, "Failed to allocate DMA.\n");
> + rc = -ENOMEM;
> + goto err_dma;
> + }
> +
> + rc = gen_pool_add_virt(ctx->vga_pool, (unsigned long)ctx->vga_virt,
> + ctx->vga_phys, ctx->vga_size, -1);
> + if (rc) {
> + dev_err(ctx->dev, "Failed to add memory to genalloc pool.\n");
> + goto err_genalloc;
> + }
> +
> + ctx->cmdq_vga_virt = gen_pool_dma_alloc(ctx->vga_pool, XDMA_CMDQ_SIZE,
> + &ctx->cmdq_vga_phys);
> + if (!ctx->cmdq_vga_virt) {
> + dev_err(ctx->dev, "Failed to genalloc cmdq.\n");
> + rc = -ENOMEM;
> + goto err_genalloc;
> + }
> +
> + dev_dbg(ctx->dev, "VGA mapped at phys[%08x], size[%08x].\n",
> + ctx->vga_phys, ctx->vga_size);
> +
> + return 0;
> +
> +err_dma:
> + dma_release_declared_memory(ctx->dev);
> +
> +err_genalloc:
> + dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
> + ctx->vga_dma);
> + return rc;
> +}
> +
> +static int aspeed_xdma_probe(struct platform_device *pdev)
> +{
> + int irq;
> + int rc;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + struct aspeed_xdma *ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->dev = dev;
> + platform_set_drvdata(pdev, ctx);
> + init_waitqueue_head(&ctx->wait);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ctx->base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ctx->base)) {
> + dev_err(dev, "Unable to ioremap registers.\n");
> + return PTR_ERR(ctx->base);
> + }
> +
> + irq = irq_of_parse_and_map(dev->of_node, 0);
> + if (!irq) {
> + dev_err(dev, "Unable to find IRQ.\n");
> + return -ENODEV;
> + }
> +
> + rc = devm_request_irq(dev, irq, aspeed_xdma_irq, IRQF_SHARED,
> + DEVICE_NAME, ctx);
> + if (rc < 0) {
> + dev_err(dev, "Unable to request IRQ %d.\n", irq);
> + return rc;
> + }
> +
> + ctx->scu = syscon_regmap_lookup_by_compatible("aspeed,ast2500-scu");
> + if (IS_ERR(ctx->scu)) {
> + dev_err(ctx->dev, "Unable to grab SCU regs.\n");
> + return PTR_ERR(ctx->scu);
> + }
> +
> + ctx->reset = devm_reset_control_get_exclusive(dev, NULL);
> + if (IS_ERR(ctx->reset)) {
> + dev_err(dev, "Unable to request reset control.\n");
> + return PTR_ERR(ctx->reset);
> + }
> +
> + ctx->vga_pool = devm_gen_pool_create(dev, ilog2(PAGE_SIZE), -1, NULL);
> + if (!ctx->vga_pool) {
> + dev_err(dev, "Unable to setup genalloc pool.\n");
> + return -ENOMEM;
> + }
> +
> + reset_control_deassert(ctx->reset);
> +
> + msleep(10);
Why 10ms again? :-)
> +
> + rc = aspeed_xdma_init_mem(ctx);
> + if (rc) {
> + reset_control_assert(ctx->reset);
> + return rc;
> + }
> +
> + aspeed_xdma_init_eng(ctx);
> +
> + return 0;
> +}
> +
> +static int aspeed_xdma_remove(struct platform_device *pdev)
> +{
> + struct aspeed_xdma *ctx = platform_get_drvdata(pdev);
> +
> + gen_pool_free(ctx->vga_pool, (unsigned long)ctx->cmdq_vga_virt,
> + XDMA_CMDQ_SIZE);
> + dma_free_coherent(ctx->dev, ctx->vga_size, ctx->vga_virt,
> + ctx->vga_dma);
> + dma_release_declared_memory(ctx->dev);
> + reset_control_assert(ctx->reset);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id aspeed_xdma_match[] = {
> + { .compatible = "aspeed,ast2500-xdma" },
> + { },
> +};
> +
> +static struct platform_driver aspeed_xdma_driver = {
> + .probe = aspeed_xdma_probe,
> + .remove = aspeed_xdma_remove,
> + .driver = {
> + .name = DEVICE_NAME,
> + .of_match_table = aspeed_xdma_match,
> + },
> +};
> +
> +module_platform_driver(aspeed_xdma_driver);
> +
> +MODULE_AUTHOR("Eddie James");
> +MODULE_DESCRIPTION("Aspeed XDMA Engine Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/uapi/linux/aspeed-xdma.h b/include/uapi/linux/aspeed-xdma.h
> new file mode 100644
> index 0000000..998459e
> --- /dev/null
> +++ b/include/uapi/linux/aspeed-xdma.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright IBM Corp 2019 */
> +
> +#ifndef _UAPI_LINUX_ASPEED_XDMA_H_
> +#define _UAPI_LINUX_ASPEED_XDMA_H_
> +
> +#include <linux/types.h>
> +
> +/*
> + * aspeed_xdma_op
> + *
> + * host_addr: the DMA address on the host side, typically configured by PCI
> + * subsystem
> + *
> + * len: the size of the transfer in bytes; it should be a multiple of 16 bytes
> + *
> + * upstream: boolean indicating the direction of the DMA operation; upstream
> + * means a transfer from the BMC to the host
> + */
> +struct aspeed_xdma_op {
> + __u64 host_addr;
> + __u32 len;
> + __u32 upstream;
> +};
> +
> +#endif /* _UAPI_LINUX_ASPEED_XDMA_H_ */
> --
> 1.8.3.1
>
--
All the best,
Eduardo Valentin
^ permalink raw reply
* [PATCH v2 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Vijay Khemka @ 2019-05-30 23:11 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530231159.222188-1-vijaykhemka@fb.com>
Added support for Infenion PXE1610 driver
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
Changes in v2:
incorporated all the feedback from Guenter Roeck <linux@roeck-us.net>
Documentation/hwmon/pxe1610 | 90 +++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
create mode 100644 Documentation/hwmon/pxe1610
diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
new file mode 100644
index 000000000000..24825db8736f
--- /dev/null
+++ b/Documentation/hwmon/pxe1610
@@ -0,0 +1,90 @@
+Kernel driver pxe1610
+=====================
+
+Supported chips:
+ * Infinion PXE1610
+ Prefix: 'pxe1610'
+ Addresses scanned: -
+ Datasheet: Datasheet is not publicly available.
+
+ * Infinion PXE1110
+ Prefix: 'pxe1110'
+ Addresses scanned: -
+ Datasheet: Datasheet is not publicly available.
+
+ * Infinion PXM1310
+ Prefix: 'pxm1310'
+ Addresses scanned: -
+ Datasheet: Datasheet is not publicly available.
+
+Author: Vijay Khemka <vijaykhemka@fb.com>
+
+
+Description
+-----------
+
+PXE1610/PXE1110 are Multi-rail/Multiphase Digital Controllers
+and compliant to
+ -- Intel VR13 DC-DC converter specifications.
+ -- Intel SVID protocol.
+Used for Vcore power regulation for Intel VR13 based microprocessors
+ -- Servers, Workstations, and High-end desktops
+
+PXM1310 is a Multi-rail Controllers and it is compliant to
+ -- Intel VR13 DC-DC converter specifications.
+ -- Intel SVID protocol.
+Used for DDR3/DDR4 Memory power regulation for Intel VR13 and
+IMVP8 based systems
+
+
+Usage Notes
+-----------
+
+This driver does not probe for PMBus devices. You will have
+to instantiate devices explicitly.
+
+Example: the following commands will load the driver for an PXE1610
+at address 0x70 on I2C bus #4:
+
+# modprobe pxe1610
+# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
+
+It can also be instantiated by declaring in device tree
+
+
+Sysfs attributes
+----------------
+
+curr1_label "iin"
+curr1_input Measured input current
+curr1_alarm Current high alarm
+
+curr[2-4]_label "iout[1-3]"
+curr[2-4]_input Measured output current
+curr[2-4]_crit Critical maximum current
+curr[2-4]_crit_alarm Current critical high alarm
+
+in1_label "vin"
+in1_input Measured input voltage
+in1_crit Critical maximum input voltage
+in1_crit_alarm Input voltage critical high alarm
+
+in[2-4]_label "vout[1-3]"
+in[2-4]_input Measured output voltage
+in[2-4]_lcrit Critical minimum output voltage
+in[2-4]_lcrit_alarm Output voltage critical low alarm
+in[2-4]_crit Critical maximum output voltage
+in[2-4]_crit_alarm Output voltage critical high alarm
+
+power1_label "pin"
+power1_input Measured input power
+power1_alarm Input power high alarm
+
+power[2-4]_label "pout[1-3]"
+power[2-4]_input Measured output power
+
+temp[1-3]_input Measured temperature
+temp[1-3]_crit Critical high temperature
+temp[1-3]_crit_alarm Chip temperature critical high alarm
+temp[1-3]_max Maximum temperature
+temp[1-3]_max_alarm Chip temperature high alarm
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Vijay Khemka @ 2019-05-30 23:11 UTC (permalink / raw)
To: linux-aspeed
Added pmbus driver for the new device Infineon pxe1610
voltage regulator. It also supports similar family device
PXE1110 and PXM1310.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
Changes in v2:
incorporated all the feedback from Guenter Roeck <linux@roeck-us.net>
drivers/hwmon/pmbus/Kconfig | 9 +++
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/pxe1610.c | 139 ++++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+)
create mode 100644 drivers/hwmon/pmbus/pxe1610.c
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 30751eb9550a..338ef9b5a395 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -154,6 +154,15 @@ config SENSORS_MAX8688
This driver can also be built as a module. If so, the module will
be called max8688.
+config SENSORS_PXE1610
+ tristate "Infineon PXE1610"
+ help
+ If you say yes here you get hardware monitoring support for Infineon
+ PXE1610.
+
+ This driver can also be built as a module. If so, the module will
+ be called pxe1610.
+
config SENSORS_TPS40422
tristate "TI TPS40422"
help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 2219b9300316..b0fbd017a91a 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
+obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
new file mode 100644
index 000000000000..ebe3f023f840
--- /dev/null
+++ b/drivers/hwmon/pmbus/pxe1610.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Hardware monitoring driver for Infineon PXE1610
+ *
+ * Copyright (c) 2019 Facebook Inc
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include "pmbus.h"
+
+#define PXE1610_NUM_PAGES 3
+
+/* Identify chip parameters. */
+static int pxe1610_identify(struct i2c_client *client,
+ struct pmbus_driver_info *info)
+{
+ if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
+ u8 vout_mode;
+ int ret;
+
+ /* Read the register with VOUT scaling value.*/
+ ret = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
+ if (ret < 0)
+ return ret;
+
+ vout_mode = ret & GENMASK(4, 0);
+
+ switch (vout_mode) {
+ case 1:
+ info->vrm_version = vr12;
+ break;
+ case 2:
+ info->vrm_version = vr13;
+ break;
+ default:
+ return -ENODEV;
+ }
+ }
+
+ return 0;
+}
+
+static struct pmbus_driver_info pxe1610_info = {
+ .pages = PXE1610_NUM_PAGES,
+ .format[PSC_VOLTAGE_IN] = linear,
+ .format[PSC_VOLTAGE_OUT] = vid,
+ .format[PSC_CURRENT_IN] = linear,
+ .format[PSC_CURRENT_OUT] = linear,
+ .format[PSC_TEMPERATURE] = linear,
+ .format[PSC_POWER] = linear,
+ .func[0] = PMBUS_HAVE_VIN
+ | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
+ | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
+ | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+ | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
+ | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
+ .func[1] = PMBUS_HAVE_VIN
+ | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
+ | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
+ | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+ | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
+ | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
+ .func[2] = PMBUS_HAVE_VIN
+ | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
+ | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
+ | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
+ | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
+ | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
+ .identify = pxe1610_identify,
+};
+
+static int pxe1610_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct pmbus_driver_info *info;
+ u8 buf[I2C_SMBUS_BLOCK_MAX];
+ int ret;
+
+ if (!i2c_check_functionality(
+ client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA
+ | I2C_FUNC_SMBUS_READ_WORD_DATA
+ | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+ return -ENODEV;
+
+ /*
+ * By default this device doesn't boot to page 0, so set page 0
+ * to access all pmbus registers.
+ */
+ i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+
+ /* Read Manufacturer id */
+ ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
+ return ret;
+ }
+ if (ret != 2 || strncmp(buf, "XP", 2)) {
+ dev_err(&client->dev, "MFR_ID unrecognized\n");
+ return -ENODEV;
+ }
+
+ info = devm_kmemdup(&client->dev, &pxe1610_info,
+ sizeof(struct pmbus_driver_info),
+ GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ return pmbus_do_probe(client, id, info);
+}
+
+static const struct i2c_device_id pxe1610_id[] = {
+ {"pxe1610", 0},
+ {"pxe1110", 0},
+ {"pxm1310", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(i2c, pxe1610_id);
+
+static struct i2c_driver pxe1610_driver = {
+ .driver = {
+ .name = "pxe1610",
+ },
+ .probe = pxe1610_probe,
+ .remove = pmbus_do_remove,
+ .id_table = pxe1610_id,
+};
+
+module_i2c_driver(pxe1610_driver);
+
+MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
+MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
+MODULE_LICENSE("GPL");
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Vijay Khemka @ 2019-05-30 20:44 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530194441.GA12310@roeck-us.net>
?On 5/30/19, 12:45 PM, "Guenter Roeck" <groeck7 at gmail.com on behalf of linux@roeck-us.net> wrote:
On Thu, May 30, 2019 at 06:51:52PM +0000, Vijay Khemka wrote:
>
>
> On 5/29/19, 6:05 PM, "Guenter Roeck" <groeck7 at gmail.com on behalf of linux@roeck-us.net> wrote:
>
> On 5/29/19 3:35 PM, Vijay Khemka wrote:
> > Added support for Infenion PXE1610 driver
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > Documentation/hwmon/pxe1610 | 84 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 84 insertions(+)
> > create mode 100644 Documentation/hwmon/pxe1610
> >
> > diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
> > new file mode 100644
> > index 000000000000..b5c83edf027a
> > --- /dev/null
> > +++ b/Documentation/hwmon/pxe1610
> > @@ -0,0 +1,84 @@
> > +Kernel driver pxe1610
> > +=====================
> > +
> > +Supported chips:
> > + * Infinion PXE1610
> > + Prefix: 'pxe1610'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > + * Infinion PXE1110
> > + Prefix: 'pxe1110'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > + * Infinion PXM1310
> > + Prefix: 'pxm1310'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > +Author: Vijay Khemka <vijaykhemka@fb.com>
> > +
> > +
> > +Description
> > +-----------
> > +
> > +PXE1610 is a Multi-rail/Multiphase Digital Controllers and
> > +it is compliant to Intel VR13 DC-DC converter specifications.
> > +
>
> And the others ?
> This supports VR12 as well and I don't see this controller supports any other VR versions.
>
The point here is that there is no description of the other controllers.
Ok, I get it, mainly all 3 controllers are from same family of Infineon controller but I will add details here.
> > +
> > +Usage Notes
> > +-----------
> > +
> > +This driver can be enabled with kernel config CONFIG_SENSORS_PXE1610
> > +set to 'y' or 'm'(for module).
> > +
> The above does not really add value.
> Ok, I will remove it.
>
> > +This driver does not probe for PMBus devices. You will have
> > +to instantiate devices explicitly.
> > +
> > +Example: the following commands will load the driver for an PXE1610
> > +at address 0x70 on I2C bus #4:
> > +
> > +# modprobe pxe1610
> > +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
> > +
> > +It can also be instantiated by declaring in device tree if it is
> > +built as a kernel not as a module.
> > +
>
> I assume you mean "built into the kernel".
> Why would devicetree based instantiation not work if the driver is built
> as module ?
> Will correct statement here.
>
> > +
> > +Sysfs attributes
> > +----------------
> > +
> > +curr1_label "iin"
> > +curr1_input Measured input current
> > +curr1_alarm Current high alarm
> > +
> > +curr[2-4]_label "iout[1-3]"
> > +curr[2-4]_input Measured output current
> > +curr[2-4]_crit Critical maximum current
> > +curr[2-4]_crit_alarm Current critical high alarm
> > +
> > +in1_label "vin"
> > +in1_input Measured input voltage
> > +in1_crit Critical maximum input voltage
> > +in1_crit_alarm Input voltage critical high alarm
> > +
> > +in[2-4]_label "vout[1-3]"
> > +in[2-4]_input Measured output voltage
> > +in[2-4]_lcrit Critical minimum output voltage
> > +in[2-4]_lcrit_alarm Output voltage critical low alarm
> > +in[2-4]_crit Critical maximum output voltage
> > +in[2-4]_crit_alarm Output voltage critical high alarm
> > +
> > +power1_label "pin"
> > +power1_input Measured input power
> > +power1_alarm Input power high alarm
> > +
> > +power[2-4]_label "pout[1-3]"
> > +power[2-4]_input Measured output power
> > +
> > +temp[1-3]_input Measured temperature
> > +temp[1-3]_crit Critical high temperature
> > +temp[1-3]_crit_alarm Chip temperature critical high alarm
> > +temp[1-3]_max Maximum temperature
> > +temp[1-3]_max_alarm Chip temperature high alarm
> >
>
>
>
^ permalink raw reply
* [PATCH] soc: aspeed: lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-05-30 20:37 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <fe36fbac-e29c-4210-9af2-defca62e9c2a@www.fastmail.com>
?On 5/29/19, 9:02 PM, "Andrew Jeffery" <andrew@aj.id.au> wrote:
On Thu, 30 May 2019, at 02:51, Vijay Khemka wrote:
> Makiing
Typo here, but I'd prefer to see this patch go in, so
Corrected in next version v2.
> memory-region and flash as optional parameter in device
> tree if user needs to use these parameter through ioctl then
> need to define in devicetree.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/soc/aspeed/aspeed-lpc-ctrl.c | 58 +++++++++++++++++-----------
> 1 file changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> index a024f8042259..aca13779764a 100644
> --- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> +++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> @@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file,
> unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> + struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
> @@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file,
> unsigned int cmd,
> if (map.window_id != 0)
> return -EINVAL;
>
> + /* If memory-region is not described in device tree */
> + if (!lpc_ctrl->mem_size) {
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + return -ENXIO;
> + }
> +
> map.size = lpc_ctrl->mem_size;
>
> return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
> @@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> return -EINVAL;
>
> if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
> + if (!lpc_ctrl->pnor_size) {
> + dev_dbg(dev, "Didn't find host pnor flash\n");
> + return -ENXIO;
> + }
> addr = lpc_ctrl->pnor_base;
> size = lpc_ctrl->pnor_size;
> } else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
> + /* If memory-region is not described in device tree */
> + if (!lpc_ctrl->mem_size) {
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + return -ENXIO;
> + }
> addr = lpc_ctrl->mem_base;
> size = lpc_ctrl->mem_size;
> } else {
> @@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
> if (!lpc_ctrl)
> return -ENOMEM;
>
> + /* If flash is described in device tree then store */
> node = of_parse_phandle(dev->of_node, "flash", 0);
> if (!node) {
> - dev_err(dev, "Didn't find host pnor flash node\n");
> - return -ENODEV;
> - }
> -
> - rc = of_address_to_resource(node, 1, &resm);
> - of_node_put(node);
> - if (rc) {
> - dev_err(dev, "Couldn't address to resource for flash\n");
> - return rc;
> + dev_dbg(dev, "Didn't find host pnor flash node\n");
> + } else {
> + rc = of_address_to_resource(node, 1, &resm);
> + of_node_put(node);
> + if (rc) {
> + dev_err(dev, "Couldn't address to resource for flash\n");
> + return rc;
> + }
> }
>
> lpc_ctrl->pnor_size = resource_size(&resm);
> @@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
>
> dev_set_drvdata(&pdev->dev, lpc_ctrl);
>
> + /* If memory-region is described in device tree then store */
> node = of_parse_phandle(dev->of_node, "memory-region", 0);
> if (!node) {
> - dev_err(dev, "Didn't find reserved memory\n");
> - return -EINVAL;
> - }
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + } else {
> + rc = of_address_to_resource(node, 0, &resm);
> + of_node_put(node);
> + if (rc) {
> + dev_err(dev, "Couldn't address to resource for reserved memory\n");
> + return -ENXIO;
> + }
>
> - rc = of_address_to_resource(node, 0, &resm);
> - of_node_put(node);
> - if (rc) {
> - dev_err(dev, "Couldn't address to resource for reserved memory\n");
> - return -ENOMEM;
> + lpc_ctrl->mem_size = resource_size(&resm);
> + lpc_ctrl->mem_base = resm.start;
> }
>
> - lpc_ctrl->mem_size = resource_size(&resm);
> - lpc_ctrl->mem_base = resm.start;
> -
> lpc_ctrl->regmap = syscon_node_to_regmap(
> pdev->dev.parent->of_node);
> if (IS_ERR(lpc_ctrl->regmap)) {
> @@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
> goto err;
> }
>
> - dev_info(dev, "Loaded at %pr\n", &resm);
> -
> return 0;
>
> err:
> --
> 2.17.1
>
>
^ permalink raw reply
* [PATCH v2] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Andrew Lunn @ 2019-05-30 20:36 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530143933.25414-1-a.filippov@yadro.com>
On Thu, May 30, 2019 at 05:39:33PM +0300, Alexander Filippov wrote:
> VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
> by YADRO.
>
> Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
> 2 files changed, 235 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 834cce80d1b8..811e9312cf22 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1259,6 +1259,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-bmc-microsoft-olympus.dtb \
> aspeed-bmc-opp-lanyang.dtb \
> aspeed-bmc-opp-palmetto.dtb \
> + aspeed-bmc-opp-vesnin.dtb \
> aspeed-bmc-opp-romulus.dtb \
> aspeed-bmc-opp-swift.dtb \
> aspeed-bmc-opp-witherspoon.dtb \
Hi Alexander
Still not correctly sorted.
Andrew
^ permalink raw reply
* [PATCH v2] soc: aspeed: lpc-ctrl: make parameter optional
From: Vijay Khemka @ 2019-05-30 20:36 UTC (permalink / raw)
To: linux-aspeed
Making memory-region and flash as optional parameter in device
tree if user needs to use these parameter through ioctl then
need to define in devicetree.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/soc/aspeed/aspeed-lpc-ctrl.c | 58 +++++++++++++++++-----------
1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
index a024f8042259..aca13779764a 100644
--- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
+++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
@@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
unsigned long param)
{
struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
+ struct device *dev = file->private_data;
void __user *p = (void __user *)param;
struct aspeed_lpc_ctrl_mapping map;
u32 addr;
@@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
if (map.window_id != 0)
return -EINVAL;
+ /* If memory-region is not described in device tree */
+ if (!lpc_ctrl->mem_size) {
+ dev_dbg(dev, "Didn't find reserved memory\n");
+ return -ENXIO;
+ }
+
map.size = lpc_ctrl->mem_size;
return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
@@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
return -EINVAL;
if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
+ if (!lpc_ctrl->pnor_size) {
+ dev_dbg(dev, "Didn't find host pnor flash\n");
+ return -ENXIO;
+ }
addr = lpc_ctrl->pnor_base;
size = lpc_ctrl->pnor_size;
} else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
+ /* If memory-region is not described in device tree */
+ if (!lpc_ctrl->mem_size) {
+ dev_dbg(dev, "Didn't find reserved memory\n");
+ return -ENXIO;
+ }
addr = lpc_ctrl->mem_base;
size = lpc_ctrl->mem_size;
} else {
@@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
if (!lpc_ctrl)
return -ENOMEM;
+ /* If flash is described in device tree then store */
node = of_parse_phandle(dev->of_node, "flash", 0);
if (!node) {
- dev_err(dev, "Didn't find host pnor flash node\n");
- return -ENODEV;
- }
-
- rc = of_address_to_resource(node, 1, &resm);
- of_node_put(node);
- if (rc) {
- dev_err(dev, "Couldn't address to resource for flash\n");
- return rc;
+ dev_dbg(dev, "Didn't find host pnor flash node\n");
+ } else {
+ rc = of_address_to_resource(node, 1, &resm);
+ of_node_put(node);
+ if (rc) {
+ dev_err(dev, "Couldn't address to resource for flash\n");
+ return rc;
+ }
}
lpc_ctrl->pnor_size = resource_size(&resm);
@@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
dev_set_drvdata(&pdev->dev, lpc_ctrl);
+ /* If memory-region is described in device tree then store */
node = of_parse_phandle(dev->of_node, "memory-region", 0);
if (!node) {
- dev_err(dev, "Didn't find reserved memory\n");
- return -EINVAL;
- }
+ dev_dbg(dev, "Didn't find reserved memory\n");
+ } else {
+ rc = of_address_to_resource(node, 0, &resm);
+ of_node_put(node);
+ if (rc) {
+ dev_err(dev, "Couldn't address to resource for reserved memory\n");
+ return -ENXIO;
+ }
- rc = of_address_to_resource(node, 0, &resm);
- of_node_put(node);
- if (rc) {
- dev_err(dev, "Couldn't address to resource for reserved memory\n");
- return -ENOMEM;
+ lpc_ctrl->mem_size = resource_size(&resm);
+ lpc_ctrl->mem_base = resm.start;
}
- lpc_ctrl->mem_size = resource_size(&resm);
- lpc_ctrl->mem_base = resm.start;
-
lpc_ctrl->regmap = syscon_node_to_regmap(
pdev->dev.parent->of_node);
if (IS_ERR(lpc_ctrl->regmap)) {
@@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
goto err;
}
- dev_info(dev, "Loaded at %pr\n", &resm);
-
return 0;
err:
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Guenter Roeck @ 2019-05-30 19:44 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <27E78CF3-FAE7-4B6F-ABD7-77F4AE1CD633@fb.com>
On Thu, May 30, 2019 at 06:51:52PM +0000, Vijay Khemka wrote:
>
>
> ?On 5/29/19, 6:05 PM, "Guenter Roeck" <groeck7 at gmail.com on behalf of linux@roeck-us.net> wrote:
>
> On 5/29/19 3:35 PM, Vijay Khemka wrote:
> > Added support for Infenion PXE1610 driver
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > Documentation/hwmon/pxe1610 | 84 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 84 insertions(+)
> > create mode 100644 Documentation/hwmon/pxe1610
> >
> > diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
> > new file mode 100644
> > index 000000000000..b5c83edf027a
> > --- /dev/null
> > +++ b/Documentation/hwmon/pxe1610
> > @@ -0,0 +1,84 @@
> > +Kernel driver pxe1610
> > +=====================
> > +
> > +Supported chips:
> > + * Infinion PXE1610
> > + Prefix: 'pxe1610'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > + * Infinion PXE1110
> > + Prefix: 'pxe1110'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > + * Infinion PXM1310
> > + Prefix: 'pxm1310'
> > + Addresses scanned: -
> > + Datasheet: Datasheet is not publicly available.
> > +
> > +Author: Vijay Khemka <vijaykhemka@fb.com>
> > +
> > +
> > +Description
> > +-----------
> > +
> > +PXE1610 is a Multi-rail/Multiphase Digital Controllers and
> > +it is compliant to Intel VR13 DC-DC converter specifications.
> > +
>
> And the others ?
> This supports VR12 as well and I don't see this controller supports any other VR versions.
>
The point here is that there is no description of the other controllers.
> > +
> > +Usage Notes
> > +-----------
> > +
> > +This driver can be enabled with kernel config CONFIG_SENSORS_PXE1610
> > +set to 'y' or 'm'(for module).
> > +
> The above does not really add value.
> Ok, I will remove it.
>
> > +This driver does not probe for PMBus devices. You will have
> > +to instantiate devices explicitly.
> > +
> > +Example: the following commands will load the driver for an PXE1610
> > +at address 0x70 on I2C bus #4:
> > +
> > +# modprobe pxe1610
> > +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
> > +
> > +It can also be instantiated by declaring in device tree if it is
> > +built as a kernel not as a module.
> > +
>
> I assume you mean "built into the kernel".
> Why would devicetree based instantiation not work if the driver is built
> as module ?
> Will correct statement here.
>
> > +
> > +Sysfs attributes
> > +----------------
> > +
> > +curr1_label "iin"
> > +curr1_input Measured input current
> > +curr1_alarm Current high alarm
> > +
> > +curr[2-4]_label "iout[1-3]"
> > +curr[2-4]_input Measured output current
> > +curr[2-4]_crit Critical maximum current
> > +curr[2-4]_crit_alarm Current critical high alarm
> > +
> > +in1_label "vin"
> > +in1_input Measured input voltage
> > +in1_crit Critical maximum input voltage
> > +in1_crit_alarm Input voltage critical high alarm
> > +
> > +in[2-4]_label "vout[1-3]"
> > +in[2-4]_input Measured output voltage
> > +in[2-4]_lcrit Critical minimum output voltage
> > +in[2-4]_lcrit_alarm Output voltage critical low alarm
> > +in[2-4]_crit Critical maximum output voltage
> > +in[2-4]_crit_alarm Output voltage critical high alarm
> > +
> > +power1_label "pin"
> > +power1_input Measured input power
> > +power1_alarm Input power high alarm
> > +
> > +power[2-4]_label "pout[1-3]"
> > +power[2-4]_input Measured output power
> > +
> > +temp[1-3]_input Measured temperature
> > +temp[1-3]_crit Critical high temperature
> > +temp[1-3]_crit_alarm Chip temperature critical high alarm
> > +temp[1-3]_max Maximum temperature
> > +temp[1-3]_max_alarm Chip temperature high alarm
> >
>
>
>
^ permalink raw reply
* [PATCH 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Vijay Khemka @ 2019-05-30 18:55 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <e72ae680-e8b7-455a-fdde-af79d429dd8c@roeck-us.net>
?On 5/29/19, 6:01 PM, "Guenter Roeck" <groeck7 at gmail.com on behalf of linux@roeck-us.net> wrote:
On 5/29/19 3:35 PM, Vijay Khemka wrote:
> Added pmbus driver for the new device Infineon pxe1610
> voltage regulator. It also supports similar family device
> PXE1110 and PXM1310.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/hwmon/pmbus/Kconfig | 9 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/pxe1610.c | 119 ++++++++++++++++++++++++++++++++++
> 3 files changed, 129 insertions(+)
> create mode 100644 drivers/hwmon/pmbus/pxe1610.c
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb9550a..338ef9b5a395 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -154,6 +154,15 @@ config SENSORS_MAX8688
> This driver can also be built as a module. If so, the module will
> be called max8688.
>
> +config SENSORS_PXE1610
> + tristate "Infineon PXE1610"
> + help
> + If you say yes here you get hardware monitoring support for Infineon
> + PXE1610.
> +
> + This driver can also be built as a module. If so, the module will
> + be called pxe1610.
> +
> config SENSORS_TPS40422
> tristate "TI TPS40422"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b9300316..b0fbd017a91a 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> +obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> new file mode 100644
> index 000000000000..01e267944df5
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon PXE1610
> + *
> + * Copyright (c) 2019 Facebook Inc
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +/*
> + * Identify chip parameters.
> + */
> +static int pxe1610_identify(struct i2c_client *client,
> + struct pmbus_driver_info *info)
Please align continuation lines with '('.
Ack.
> +{
> + if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
> + int vout_mode;
> +
> + vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
pmbus_read_byte_data() can return an error. Calling pmbus_check_byte_register()
doesn't really add any value here, since the second call can still fail,
which needs to be checked.
Ack.
> + switch (vout_mode & 0x1f) {
> + case 1:
> + info->vrm_version = vr12;
> + break;
Alignment is off.
Ack.
> + case 2:
> + info->vrm_version = vr13;
> + break;
Same here.
> + default:
> + return -ENODEV;
> + }
> + }
> + return 0;
> +}
> +
> +static int pxe1610_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + /* By default this device doesn't boot to page 0, so set page 0
> + * to access all pmbus registers.
> + */
Please use standard multi-line comments.
Ack.
> + i2c_smbus_write_byte_data(client, 0, 0);
> +
Please use the PMBUS_PAGE command definition.
Will use.
I wonder if it would make sense to initialize currpage in the core to an unreasonable
number for multi-page chips, but I guess that is a different question.
> + /* Read Manufacturer id */
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> + if (ret != 2 || strncmp(buf, "XP", strlen("XP"))) {
The strlen() is really unnecessary here. Just use 2 (and a define
for it if you like).
Ack
> + dev_err(&client->dev, "MFR_ID unrecognised\n");
unrecognized. Oh well, turns out unrecognised is the British spelling and
just as valid, so feel free to keep it if you like.
> + return -ENODEV;
> + }
> +
> + info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = vid;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> +
> + info->func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP;
> + info->func[1] = info->func[0];
> + info->func[2] = info->func[0];
> +
> + info->pages = id->driver_data;
> + info->identify = pxe1610_identify;
> +
It doesn't really add value to initialize all these parameters manually.
I would suggest to use the approach from tps53679.c, ie have a static
structure and use devm_kmemdup() to pass a copy to pmbus_do_probe().
Will look into this.
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id pxe1610_id[] = {
> + {"pxe1610", 3},
> + {"pxe1110", 3},
> + {"pxm1310", 3},
Unless there are chips with different page counts in the queue, using
driver_data to pass the number of pages does not add any value. Just
set num_pages to 3.
If you like, feel free to use a define instead of a constant.
Ack.
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pxe1610_id);
> +
> +/* This is the driver that will be inserted */
This comment does not add any value.
> +static struct i2c_driver pxe1610_driver = {
> + .driver = {
> + .name = "pxe1610",
> + },
> + .probe = pxe1610_probe,
> + .remove = pmbus_do_remove,
> + .id_table = pxe1610_id,
> +};
> +
> +module_i2c_driver(pxe1610_driver);
> +
> +MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
* [PATCH 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Vijay Khemka @ 2019-05-30 18:51 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <0a94e784-41a0-4f2d-f9f8-6b365a1e755e@roeck-us.net>
?On 5/29/19, 6:05 PM, "Guenter Roeck" <groeck7 at gmail.com on behalf of linux@roeck-us.net> wrote:
On 5/29/19 3:35 PM, Vijay Khemka wrote:
> Added support for Infenion PXE1610 driver
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> Documentation/hwmon/pxe1610 | 84 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> create mode 100644 Documentation/hwmon/pxe1610
>
> diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
> new file mode 100644
> index 000000000000..b5c83edf027a
> --- /dev/null
> +++ b/Documentation/hwmon/pxe1610
> @@ -0,0 +1,84 @@
> +Kernel driver pxe1610
> +=====================
> +
> +Supported chips:
> + * Infinion PXE1610
> + Prefix: 'pxe1610'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXE1110
> + Prefix: 'pxe1110'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXM1310
> + Prefix: 'pxm1310'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> +Author: Vijay Khemka <vijaykhemka@fb.com>
> +
> +
> +Description
> +-----------
> +
> +PXE1610 is a Multi-rail/Multiphase Digital Controllers and
> +it is compliant to Intel VR13 DC-DC converter specifications.
> +
And the others ?
This supports VR12 as well and I don't see this controller supports any other VR versions.
> +
> +Usage Notes
> +-----------
> +
> +This driver can be enabled with kernel config CONFIG_SENSORS_PXE1610
> +set to 'y' or 'm'(for module).
> +
The above does not really add value.
Ok, I will remove it.
> +This driver does not probe for PMBus devices. You will have
> +to instantiate devices explicitly.
> +
> +Example: the following commands will load the driver for an PXE1610
> +at address 0x70 on I2C bus #4:
> +
> +# modprobe pxe1610
> +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
> +
> +It can also be instantiated by declaring in device tree if it is
> +built as a kernel not as a module.
> +
I assume you mean "built into the kernel".
Why would devicetree based instantiation not work if the driver is built
as module ?
Will correct statement here.
> +
> +Sysfs attributes
> +----------------
> +
> +curr1_label "iin"
> +curr1_input Measured input current
> +curr1_alarm Current high alarm
> +
> +curr[2-4]_label "iout[1-3]"
> +curr[2-4]_input Measured output current
> +curr[2-4]_crit Critical maximum current
> +curr[2-4]_crit_alarm Current critical high alarm
> +
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_crit Critical maximum input voltage
> +in1_crit_alarm Input voltage critical high alarm
> +
> +in[2-4]_label "vout[1-3]"
> +in[2-4]_input Measured output voltage
> +in[2-4]_lcrit Critical minimum output voltage
> +in[2-4]_lcrit_alarm Output voltage critical low alarm
> +in[2-4]_crit Critical maximum output voltage
> +in[2-4]_crit_alarm Output voltage critical high alarm
> +
> +power1_label "pin"
> +power1_input Measured input power
> +power1_alarm Input power high alarm
> +
> +power[2-4]_label "pout[1-3]"
> +power[2-4]_input Measured output power
> +
> +temp[1-3]_input Measured temperature
> +temp[1-3]_crit Critical high temperature
> +temp[1-3]_crit_alarm Chip temperature critical high alarm
> +temp[1-3]_max Maximum temperature
> +temp[1-3]_max_alarm Chip temperature high alarm
>
^ permalink raw reply
* [PATCH v2] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Andrew Lunn @ 2019-05-30 16:53 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530143933.25414-1-a.filippov@yadro.com>
On Thu, May 30, 2019 at 05:39:33PM +0300, Alexander Filippov wrote:
> VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
> by YADRO.
>
> Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
> 2 files changed, 235 insertions(+)
> create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 834cce80d1b8..811e9312cf22 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1259,6 +1259,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> aspeed-bmc-microsoft-olympus.dtb \
> aspeed-bmc-opp-lanyang.dtb \
> aspeed-bmc-opp-palmetto.dtb \
> + aspeed-bmc-opp-vesnin.dtb \
> aspeed-bmc-opp-romulus.dtb \
> aspeed-bmc-opp-swift.dtb \
> aspeed-bmc-opp-witherspoon.dtb \
Hi Alexander
These appear to be in alphabetic order, so it should be added before
witherspoon.
Andrew
^ permalink raw reply
* [PATCH v2] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Alexander Filippov @ 2019-05-30 14:39 UTC (permalink / raw)
To: linux-aspeed
VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
by YADRO.
Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 234 ++++++++++++++++++++
2 files changed, 235 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 834cce80d1b8..811e9312cf22 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1259,6 +1259,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-microsoft-olympus.dtb \
aspeed-bmc-opp-lanyang.dtb \
aspeed-bmc-opp-palmetto.dtb \
+ aspeed-bmc-opp-vesnin.dtb \
aspeed-bmc-opp-romulus.dtb \
aspeed-bmc-opp-swift.dtb \
aspeed-bmc-opp-witherspoon.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
new file mode 100644
index 000000000000..20f07f5bb4f4
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright 2019 YADRO
+/dts-v1/;
+
+#include "aspeed-g4.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+
+/ {
+ model = "Vesnin BMC";
+ compatible = "yadro,vesnin-bmc", "aspeed,ast2400";
+
+ chosen {
+ stdout-path = &uart5;
+ bootargs = "console=ttyS4,115200 earlyprintk";
+ };
+
+ memory {
+ reg = <0x40000000 0x20000000>;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ vga_memory: framebuffer at 5f000000 {
+ no-map;
+ reg = <0x5f000000 0x01000000>; /* 16MB */
+ };
+ flash_memory: region at 5c000000 {
+ no-map;
+ reg = <0x5c000000 0x02000000>; /* 32M */
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ heartbeat {
+ gpios = <&gpio ASPEED_GPIO(R, 4) GPIO_ACTIVE_LOW>;
+ };
+ power_red {
+ gpios = <&gpio ASPEED_GPIO(N, 1) GPIO_ACTIVE_LOW>;
+ };
+
+ id_blue {
+ gpios = <&gpio ASPEED_GPIO(O, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_red {
+ gpios = <&gpio ASPEED_GPIO(N, 6) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_yel {
+ gpios = <&gpio ASPEED_GPIO(N, 7) GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ button_checkstop {
+ label = "checkstop";
+ linux,code = <74>;
+ gpios = <&gpio ASPEED_GPIO(P, 5) GPIO_ACTIVE_LOW>;
+ };
+
+ button_identify {
+ label = "identify";
+ linux,code = <152>;
+ gpios = <&gpio ASPEED_GPIO(O, 7) GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&fmc {
+ status = "okay";
+ flash at 0 {
+ status = "okay";
+ m25p,fast-read;
+ label = "bmc";
+#include "openbmc-flash-layout.dtsi"
+ };
+};
+
+&spi {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1debug_default>;
+
+ flash at 0 {
+ status = "okay";
+ label = "pnor";
+ m25p,fast-read;
+ };
+};
+
+&mac0 {
+ status = "okay";
+
+ use-ncsi;
+ no-hw-checksum;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rmii1_default>;
+};
+
+
+&uart5 {
+ status = "okay";
+};
+
+&lpc_ctrl {
+ status = "okay";
+ memory-region = <&flash_memory>;
+ flash = <&spi>;
+};
+
+&ibt {
+ status = "okay";
+};
+
+&lpc_host {
+ sio_regs: regs {
+ compatible = "aspeed,bmc-misc";
+ };
+};
+
+&mbox {
+ status = "okay";
+};
+
+&uart3 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
+};
+
+&i2c0 {
+ status = "okay";
+
+ eeprom at 50 {
+ compatible = "atmel,24c256";
+ reg = <0x50>;
+ pagesize = <64>;
+ };
+};
+
+&i2c1 {
+ status = "okay";
+
+ tmp75 at 49 {
+ compatible = "ti,tmp75";
+ reg = <0x49>;
+ };
+};
+
+&i2c2 {
+ status = "okay";
+};
+
+&i2c3 {
+ status = "okay";
+};
+
+&i2c4 {
+ status = "okay";
+
+ occ-hwmon at 50 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x50>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+
+ occ-hwmon at 51 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x51>;
+ };
+};
+
+&i2c6 {
+ status = "okay";
+
+ w83795g at 2f {
+ compatible = "nuvoton,w83795g";
+ reg = <0x2f>;
+ };
+};
+
+&i2c7 {
+ status = "okay";
+
+ occ-hwmon at 56 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x56>;
+ };
+};
+
+&i2c9 {
+ status = "okay";
+};
+
+&i2c10 {
+ status = "okay";
+};
+
+&i2c11 {
+ status = "okay";
+
+ occ-hwmon at 57 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x57>;
+ };
+};
+
+&i2c12 {
+ status = "okay";
+
+ rtc at 68 {
+ compatible = "maxim,ds3231";
+ reg = <0x68>;
+ };
+};
+
+&i2c13 {
+ status = "okay";
+};
+
+&vuart {
+ status = "okay";
+};
--
2.20.1
^ permalink raw reply related
* [PATCH] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Alexander A. Filippov @ 2019-05-30 14:31 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <CACPK8XfG7j4Z2bqX9CFxUeUrpx708Uqbh-5ts9W5SnDfDw-xYA@mail.gmail.com>
On Thu, May 30, 2019 at 02:16:59PM +0000, Joel Stanley wrote:
> On Thu, 30 May 2019 at 09:40, Alexander Filippov <a.filippov@yadro.com> wrote:
> > @@ -0,0 +1,262 @@
>
> Can we get a SDPX license string at the top of the file? Something like this:
>
> // SPDX-License-Identifier: GPL-2.0+
> // Copyright 2019 <copyright holder>
Sure, on my way.
>
> > +/dts-v1/;
> > +
> > +#include "aspeed-g4.dtsi"
> > +#include <dt-bindings/gpio/aspeed-gpio.h>
> > +
>
> > +&i2c3 {
> > + status = "okay";
> > + cpr2021 at 59 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + compatible = "general,cpr2021", "general,pmbus";
>
> Do you have a driver for this one you plan on submitting?
Yes, we plan but not right now. I remove it now and it will be added when the
driver will be ready.
^ permalink raw reply
* [PATCH] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Joel Stanley @ 2019-05-30 14:16 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190530093948.12479-1-a.filippov@yadro.com>
On Thu, 30 May 2019 at 09:40, Alexander Filippov <a.filippov@yadro.com> wrote:
> @@ -0,0 +1,262 @@
Can we get a SDPX license string at the top of the file? Something like this:
// SPDX-License-Identifier: GPL-2.0+
// Copyright 2019 <copyright holder>
> +/dts-v1/;
> +
> +#include "aspeed-g4.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +
> +&i2c3 {
> + status = "okay";
> + cpr2021 at 59 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "general,cpr2021", "general,pmbus";
Do you have a driver for this one you plan on submitting?
^ permalink raw reply
* [PATCH] ARM: dts: aspeed: Add YADRO VESNIN BMC
From: Alexander Filippov @ 2019-05-30 9:39 UTC (permalink / raw)
To: linux-aspeed
VESNIN is an OpenPower machine with an Aspeed 2400 BMC SoC manufactured
by YADRO.
Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts | 262 ++++++++++++++++++++
2 files changed, 263 insertions(+)
create mode 100644 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 834cce80d1b8..811e9312cf22 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1259,6 +1259,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
aspeed-bmc-microsoft-olympus.dtb \
aspeed-bmc-opp-lanyang.dtb \
aspeed-bmc-opp-palmetto.dtb \
+ aspeed-bmc-opp-vesnin.dtb \
aspeed-bmc-opp-romulus.dtb \
aspeed-bmc-opp-swift.dtb \
aspeed-bmc-opp-witherspoon.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
new file mode 100644
index 000000000000..ac0fd525adbd
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts
@@ -0,0 +1,262 @@
+/dts-v1/;
+
+#include "aspeed-g4.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+
+/ {
+ model = "Vesnin BMC";
+ compatible = "yadro,vesnin-bmc", "aspeed,ast2400";
+
+ chosen {
+ stdout-path = &uart5;
+ bootargs = "console=ttyS4,115200 earlyprintk";
+ };
+
+ memory {
+ reg = <0x40000000 0x20000000>;
+ };
+
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ vga_memory: framebuffer at 5f000000 {
+ no-map;
+ reg = <0x5f000000 0x01000000>; /* 16MB */
+ };
+ flash_memory: region at 5c000000 {
+ no-map;
+ reg = <0x5c000000 0x02000000>; /* 32M */
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ heartbeat {
+ gpios = <&gpio ASPEED_GPIO(R, 4) GPIO_ACTIVE_LOW>;
+ };
+ power_red {
+ gpios = <&gpio ASPEED_GPIO(N, 1) GPIO_ACTIVE_LOW>;
+ };
+
+ id_blue {
+ gpios = <&gpio ASPEED_GPIO(O, 0) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_red {
+ gpios = <&gpio ASPEED_GPIO(N, 6) GPIO_ACTIVE_LOW>;
+ };
+
+ alarm_yel {
+ gpios = <&gpio ASPEED_GPIO(N, 7) GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+
+ button_checkstop {
+ label = "checkstop";
+ linux,code = <74>;
+ gpios = <&gpio ASPEED_GPIO(P, 5) GPIO_ACTIVE_LOW>;
+ };
+
+ button_identify {
+ label = "identify";
+ linux,code = <152>;
+ gpios = <&gpio ASPEED_GPIO(O, 7) GPIO_ACTIVE_LOW>;
+ };
+ };
+};
+
+&fmc {
+ status = "okay";
+ flash at 0 {
+ status = "okay";
+ m25p,fast-read;
+ label = "bmc";
+#include "openbmc-flash-layout.dtsi"
+ };
+};
+
+&spi {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_spi1debug_default>;
+
+ flash at 0 {
+ status = "okay";
+ label = "pnor";
+ m25p,fast-read;
+ };
+};
+
+&mac0 {
+ status = "okay";
+
+ use-ncsi;
+ no-hw-checksum;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_rmii1_default>;
+};
+
+
+&uart5 {
+ status = "okay";
+};
+
+&lpc_ctrl {
+ status = "okay";
+ memory-region = <&flash_memory>;
+ flash = <&spi>;
+};
+
+&ibt {
+ status = "okay";
+};
+
+&lpc_host {
+ sio_regs: regs {
+ compatible = "aspeed,bmc-misc";
+ };
+};
+
+&mbox {
+ status = "okay";
+};
+
+&uart3 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_txd2_default &pinctrl_rxd2_default>;
+};
+
+&i2c0 {
+ status = "okay";
+
+ eeprom at 50 {
+ compatible = "atmel,24c256";
+ reg = <0x50>;
+ pagesize = <64>;
+ };
+};
+
+&i2c1 {
+ status = "okay";
+
+ tmp75 at 49 {
+ compatible = "ti,tmp75";
+ reg = <0x49>;
+ };
+};
+
+&i2c2 {
+ status = "okay";
+};
+
+&i2c3 {
+ status = "okay";
+ cpr2021 at 59 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "general,cpr2021", "general,pmbus";
+ reg = <0x59>;
+ present {
+ gpios = <&gpio ASPEED_GPIO(P, 7) GPIO_ACTIVE_LOW>;
+ };
+ smbalert {
+ gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
+ };
+ pwok {
+ gpios = <&gpio ASPEED_GPIO(E, 6) GPIO_ACTIVE_HIGH>;
+ };
+ };
+ cpr2021 at 5a {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "general,cpr2021", "general,pmbus";
+ reg = <0x5a>;
+ present {
+ gpios = <&gpio ASPEED_GPIO(N, 0) GPIO_ACTIVE_LOW>;
+ };
+ smbalert {
+ gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
+ };
+ pwok {
+ gpios = <&gpio ASPEED_GPIO(E, 6) GPIO_ACTIVE_HIGH>;
+ };
+ };
+};
+
+&i2c4 {
+ status = "okay";
+
+ occ-hwmon at 50 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x50>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+
+ occ-hwmon at 51 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x51>;
+ };
+};
+
+&i2c6 {
+ status = "okay";
+
+ w83795g at 2f {
+ compatible = "nuvoton,w83795g";
+ reg = <0x2f>;
+ };
+};
+
+&i2c7 {
+ status = "okay";
+
+ occ-hwmon at 56 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x56>;
+ };
+};
+
+&i2c9 {
+ status = "okay";
+};
+
+&i2c10 {
+ status = "okay";
+};
+
+&i2c11 {
+ status = "okay";
+
+ occ-hwmon at 57 {
+ compatible = "ibm,p8-occ-hwmon";
+ reg = <0x57>;
+ };
+};
+
+&i2c12 {
+ status = "okay";
+
+ rtc at 68 {
+ compatible = "maxim,ds3231";
+ reg = <0x68>;
+ };
+};
+
+&i2c13 {
+ status = "okay";
+};
+
+&vuart {
+ status = "okay";
+};
--
2.20.1
^ permalink raw reply related
* [PATCH v2] ARM: dts: aspeed: g4: add video engine support
From: Alexander Filippov @ 2019-05-30 9:35 UTC (permalink / raw)
To: linux-aspeed
Add a node to describe the video engine on AST2400.
These changes were copied from aspeed-g5.dtsi
Signed-off-by: Alexander Filippov <a.filippov@yadro.com>
---
arch/arm/boot/dts/aspeed-g4.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 6011692df15a..5a9e3f684359 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -195,6 +195,16 @@
reg = <0x1e720000 0x8000>; // 32K
};
+ video: video at 1e700000 {
+ compatible = "aspeed,ast2400-video-engine";
+ reg = <0x1e700000 0x1000>;
+ clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
+ <&syscon ASPEED_CLK_GATE_ECLK>;
+ clock-names = "vclk", "eclk";
+ interrupts = <7>;
+ status = "disabled";
+ };
+
gpio: gpio at 1e780000 {
#gpio-cells = <2>;
gpio-controller;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v3 1/8] dt-bindings: soc: Add Aspeed XDMA engine binding documentation
From: Andrew Jeffery @ 2019-05-30 5:30 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <1559153408-31190-2-git-send-email-eajames@linux.ibm.com>
On Thu, 30 May 2019, at 03:40, Eddie James wrote:
> Document the bindings.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> .../devicetree/bindings/soc/aspeed/xdma.txt | 23 ++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/aspeed/xdma.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> new file mode 100644
> index 0000000..85e82ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/aspeed/xdma.txt
> @@ -0,0 +1,23 @@
> +* Device tree bindings for the Aspeed XDMA Engine
> +
> +The XDMA Engine embedded in the AST2500 SOC can perform automatic DMA
> +operations over PCI between the AST2500 (acting as a BMC) and a host
> processor.
> +
> +Required properties:
> +
> + - compatible "aspeed,ast2500-xdma"
> + - reg contains the offset and length of the memory region
> + assigned to the XDMA registers
> + - resets reset specifier for the syscon reset associated with
> + the XDMA engine
> + - interrupts the interrupt associated with the XDMA engine on this
> + platform
The indentation is quite distracting. If you rev the series can you fix it?
Otherwise,
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> +
> +Example:
> +
> + xdma at 1e6e7000 {
> + compatible = "aspeed,ast2500-xdma";
> + reg = <0x1e6e7000 0x100>;
> + resets = <&syscon ASPEED_RESET_XDMA>;
> + interrupts = <6>;
> + };
> --
> 1.8.3.1
>
>
^ permalink raw reply
* [PATCH] soc: aspeed: lpc-ctrl: make parameter optional
From: Andrew Jeffery @ 2019-05-30 4:02 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190529172103.1130525-1-vijaykhemka@fb.com>
On Thu, 30 May 2019, at 02:51, Vijay Khemka wrote:
> Makiing
Typo here, but I'd prefer to see this patch go in, so
> memory-region and flash as optional parameter in device
> tree if user needs to use these parameter through ioctl then
> need to define in devicetree.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> drivers/soc/aspeed/aspeed-lpc-ctrl.c | 58 +++++++++++++++++-----------
> 1 file changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> index a024f8042259..aca13779764a 100644
> --- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> +++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
> @@ -68,6 +68,7 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file,
> unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> + struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
> @@ -90,6 +91,12 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file,
> unsigned int cmd,
> if (map.window_id != 0)
> return -EINVAL;
>
> + /* If memory-region is not described in device tree */
> + if (!lpc_ctrl->mem_size) {
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + return -ENXIO;
> + }
> +
> map.size = lpc_ctrl->mem_size;
>
> return copy_to_user(p, &map, sizeof(map)) ? -EFAULT : 0;
> @@ -126,9 +133,18 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> return -EINVAL;
>
> if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
> + if (!lpc_ctrl->pnor_size) {
> + dev_dbg(dev, "Didn't find host pnor flash\n");
> + return -ENXIO;
> + }
> addr = lpc_ctrl->pnor_base;
> size = lpc_ctrl->pnor_size;
> } else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
> + /* If memory-region is not described in device tree */
> + if (!lpc_ctrl->mem_size) {
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + return -ENXIO;
> + }
> addr = lpc_ctrl->mem_base;
> size = lpc_ctrl->mem_size;
> } else {
> @@ -196,17 +212,17 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
> if (!lpc_ctrl)
> return -ENOMEM;
>
> + /* If flash is described in device tree then store */
> node = of_parse_phandle(dev->of_node, "flash", 0);
> if (!node) {
> - dev_err(dev, "Didn't find host pnor flash node\n");
> - return -ENODEV;
> - }
> -
> - rc = of_address_to_resource(node, 1, &resm);
> - of_node_put(node);
> - if (rc) {
> - dev_err(dev, "Couldn't address to resource for flash\n");
> - return rc;
> + dev_dbg(dev, "Didn't find host pnor flash node\n");
> + } else {
> + rc = of_address_to_resource(node, 1, &resm);
> + of_node_put(node);
> + if (rc) {
> + dev_err(dev, "Couldn't address to resource for flash\n");
> + return rc;
> + }
> }
>
> lpc_ctrl->pnor_size = resource_size(&resm);
> @@ -214,22 +230,22 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
>
> dev_set_drvdata(&pdev->dev, lpc_ctrl);
>
> + /* If memory-region is described in device tree then store */
> node = of_parse_phandle(dev->of_node, "memory-region", 0);
> if (!node) {
> - dev_err(dev, "Didn't find reserved memory\n");
> - return -EINVAL;
> - }
> + dev_dbg(dev, "Didn't find reserved memory\n");
> + } else {
> + rc = of_address_to_resource(node, 0, &resm);
> + of_node_put(node);
> + if (rc) {
> + dev_err(dev, "Couldn't address to resource for reserved memory\n");
> + return -ENXIO;
> + }
>
> - rc = of_address_to_resource(node, 0, &resm);
> - of_node_put(node);
> - if (rc) {
> - dev_err(dev, "Couldn't address to resource for reserved memory\n");
> - return -ENOMEM;
> + lpc_ctrl->mem_size = resource_size(&resm);
> + lpc_ctrl->mem_base = resm.start;
> }
>
> - lpc_ctrl->mem_size = resource_size(&resm);
> - lpc_ctrl->mem_base = resm.start;
> -
> lpc_ctrl->regmap = syscon_node_to_regmap(
> pdev->dev.parent->of_node);
> if (IS_ERR(lpc_ctrl->regmap)) {
> @@ -258,8 +274,6 @@ static int aspeed_lpc_ctrl_probe(struct
> platform_device *pdev)
> goto err;
> }
>
> - dev_info(dev, "Loaded at %pr\n", &resm);
> -
> return 0;
>
> err:
> --
> 2.17.1
>
>
^ permalink raw reply
* [PATCH 2/2] Docs: hwmon: pmbus: Add PXE1610 driver
From: Guenter Roeck @ 2019-05-30 1:05 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190529223511.4059120-2-vijaykhemka@fb.com>
On 5/29/19 3:35 PM, Vijay Khemka wrote:
> Added support for Infenion PXE1610 driver
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> Documentation/hwmon/pxe1610 | 84 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> create mode 100644 Documentation/hwmon/pxe1610
>
> diff --git a/Documentation/hwmon/pxe1610 b/Documentation/hwmon/pxe1610
> new file mode 100644
> index 000000000000..b5c83edf027a
> --- /dev/null
> +++ b/Documentation/hwmon/pxe1610
> @@ -0,0 +1,84 @@
> +Kernel driver pxe1610
> +=====================
> +
> +Supported chips:
> + * Infinion PXE1610
> + Prefix: 'pxe1610'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXE1110
> + Prefix: 'pxe1110'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> + * Infinion PXM1310
> + Prefix: 'pxm1310'
> + Addresses scanned: -
> + Datasheet: Datasheet is not publicly available.
> +
> +Author: Vijay Khemka <vijaykhemka@fb.com>
> +
> +
> +Description
> +-----------
> +
> +PXE1610 is a Multi-rail/Multiphase Digital Controllers and
> +it is compliant to Intel VR13 DC-DC converter specifications.
> +
And the others ?
> +
> +Usage Notes
> +-----------
> +
> +This driver can be enabled with kernel config CONFIG_SENSORS_PXE1610
> +set to 'y' or 'm'(for module).
> +
The above does not really add value.
> +This driver does not probe for PMBus devices. You will have
> +to instantiate devices explicitly.
> +
> +Example: the following commands will load the driver for an PXE1610
> +at address 0x70 on I2C bus #4:
> +
> +# modprobe pxe1610
> +# echo pxe1610 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
> +
> +It can also be instantiated by declaring in device tree if it is
> +built as a kernel not as a module.
> +
I assume you mean "built into the kernel".
Why would devicetree based instantiation not work if the driver is built
as module ?
> +
> +Sysfs attributes
> +----------------
> +
> +curr1_label "iin"
> +curr1_input Measured input current
> +curr1_alarm Current high alarm
> +
> +curr[2-4]_label "iout[1-3]"
> +curr[2-4]_input Measured output current
> +curr[2-4]_crit Critical maximum current
> +curr[2-4]_crit_alarm Current critical high alarm
> +
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_crit Critical maximum input voltage
> +in1_crit_alarm Input voltage critical high alarm
> +
> +in[2-4]_label "vout[1-3]"
> +in[2-4]_input Measured output voltage
> +in[2-4]_lcrit Critical minimum output voltage
> +in[2-4]_lcrit_alarm Output voltage critical low alarm
> +in[2-4]_crit Critical maximum output voltage
> +in[2-4]_crit_alarm Output voltage critical high alarm
> +
> +power1_label "pin"
> +power1_input Measured input power
> +power1_alarm Input power high alarm
> +
> +power[2-4]_label "pout[1-3]"
> +power[2-4]_input Measured output power
> +
> +temp[1-3]_input Measured temperature
> +temp[1-3]_crit Critical high temperature
> +temp[1-3]_crit_alarm Chip temperature critical high alarm
> +temp[1-3]_max Maximum temperature
> +temp[1-3]_max_alarm Chip temperature high alarm
>
^ permalink raw reply
* [PATCH 1/2] hwmon: pmbus: Add Infineon PXE1610 VR driver
From: Guenter Roeck @ 2019-05-30 1:00 UTC (permalink / raw)
To: linux-aspeed
In-Reply-To: <20190529223511.4059120-1-vijaykhemka@fb.com>
On 5/29/19 3:35 PM, Vijay Khemka wrote:
> Added pmbus driver for the new device Infineon pxe1610
> voltage regulator. It also supports similar family device
> PXE1110 and PXM1310.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/hwmon/pmbus/Kconfig | 9 +++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/pxe1610.c | 119 ++++++++++++++++++++++++++++++++++
> 3 files changed, 129 insertions(+)
> create mode 100644 drivers/hwmon/pmbus/pxe1610.c
>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index 30751eb9550a..338ef9b5a395 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -154,6 +154,15 @@ config SENSORS_MAX8688
> This driver can also be built as a module. If so, the module will
> be called max8688.
>
> +config SENSORS_PXE1610
> + tristate "Infineon PXE1610"
> + help
> + If you say yes here you get hardware monitoring support for Infineon
> + PXE1610.
> +
> + This driver can also be built as a module. If so, the module will
> + be called pxe1610.
> +
> config SENSORS_TPS40422
> tristate "TI TPS40422"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 2219b9300316..b0fbd017a91a 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
> obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
> obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
> obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
> +obj-$(CONFIG_SENSORS_PXE1610) += pxe1610.o
> obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
> obj-$(CONFIG_SENSORS_TPS53679) += tps53679.o
> obj-$(CONFIG_SENSORS_UCD9000) += ucd9000.o
> diff --git a/drivers/hwmon/pmbus/pxe1610.c b/drivers/hwmon/pmbus/pxe1610.c
> new file mode 100644
> index 000000000000..01e267944df5
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/pxe1610.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Hardware monitoring driver for Infineon PXE1610
> + *
> + * Copyright (c) 2019 Facebook Inc
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include "pmbus.h"
> +
> +/*
> + * Identify chip parameters.
> + */
> +static int pxe1610_identify(struct i2c_client *client,
> + struct pmbus_driver_info *info)
Please align continuation lines with '('.
> +{
> + if (pmbus_check_byte_register(client, 0, PMBUS_VOUT_MODE)) {
> + int vout_mode;
> +
> + vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
pmbus_read_byte_data() can return an error. Calling pmbus_check_byte_register()
doesn't really add any value here, since the second call can still fail,
which needs to be checked.
> + switch (vout_mode & 0x1f) {
> + case 1:
> + info->vrm_version = vr12;
> + break;
Alignment is off.
> + case 2:
> + info->vrm_version = vr13;
> + break;
Same here.
> + default:
> + return -ENODEV;
> + }
> + }
> + return 0;
> +}
> +
> +static int pxe1610_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u8 buf[I2C_SMBUS_BLOCK_MAX];
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA
> + | I2C_FUNC_SMBUS_READ_WORD_DATA
> + | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> + return -ENODEV;
> +
> + /* By default this device doesn't boot to page 0, so set page 0
> + * to access all pmbus registers.
> + */
Please use standard multi-line comments.
> + i2c_smbus_write_byte_data(client, 0, 0);
> +
Please use the PMBUS_PAGE command definition.
I wonder if it would make sense to initialize currpage in the core to an unreasonable
number for multi-page chips, but I guess that is a different question.
> + /* Read Manufacturer id */
> + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, buf);
> + if (ret < 0) {
> + dev_err(&client->dev, "Failed to read PMBUS_MFR_ID\n");
> + return ret;
> + }
> + if (ret != 2 || strncmp(buf, "XP", strlen("XP"))) {
The strlen() is really unnecessary here. Just use 2 (and a define
for it if you like).
> + dev_err(&client->dev, "MFR_ID unrecognised\n");
unrecognized. Oh well, turns out unrecognised is the British spelling and
just as valid, so feel free to keep it if you like.
> + return -ENODEV;
> + }
> +
> + info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->format[PSC_VOLTAGE_IN] = linear;
> + info->format[PSC_VOLTAGE_OUT] = vid;
> + info->format[PSC_CURRENT_IN] = linear;
> + info->format[PSC_CURRENT_OUT] = linear;
> + info->format[PSC_POWER] = linear;
> + info->format[PSC_TEMPERATURE] = linear;
> +
> + info->func[0] = PMBUS_HAVE_VIN
> + | PMBUS_HAVE_VOUT | PMBUS_HAVE_IIN
> + | PMBUS_HAVE_IOUT | PMBUS_HAVE_PIN
> + | PMBUS_HAVE_POUT | PMBUS_HAVE_TEMP
> + | PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT
> + | PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP;
> + info->func[1] = info->func[0];
> + info->func[2] = info->func[0];
> +
> + info->pages = id->driver_data;
> + info->identify = pxe1610_identify;
> +
It doesn't really add value to initialize all these parameters manually.
I would suggest to use the approach from tps53679.c, ie have a static
structure and use devm_kmemdup() to pass a copy to pmbus_do_probe().
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct i2c_device_id pxe1610_id[] = {
> + {"pxe1610", 3},
> + {"pxe1110", 3},
> + {"pxm1310", 3},
Unless there are chips with different page counts in the queue, using
driver_data to pass the number of pages does not add any value. Just
set num_pages to 3.
If you like, feel free to use a define instead of a constant.
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pxe1610_id);
> +
> +/* This is the driver that will be inserted */
This comment does not add any value.
> +static struct i2c_driver pxe1610_driver = {
> + .driver = {
> + .name = "pxe1610",
> + },
> + .probe = pxe1610_probe,
> + .remove = pmbus_do_remove,
> + .id_table = pxe1610_id,
> +};
> +
> +module_i2c_driver(pxe1610_driver);
> +
> +MODULE_AUTHOR("Vijay Khemka <vijaykhemka@fb.com>");
> +MODULE_DESCRIPTION("PMBus driver for Infineon PXE1610, PXE1110 and PXM1310");
> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox