[Pdbg] [PATCH 1/3] enable libpdbg to use i2ctools lib
Alistair Popple
alistair at popple.id.au
Wed Apr 10 12:11:18 AEST 2019
Thanks Rashmica, couple of comments below...
> +
> +#define MAX_I2C_BUSES 100
> +static int i2c_buses[100] = {0};
You shouldn't need this as you should be storing the data in the target itself
(ie. the struct i2cm) rather than creating your own index.
> +static int kernel_i2c_get(struct i2cm *i2cm, uint32_t addr, uint32_t
> offset, uint16_t size, uint64_t *data) +{
> + int res = 0;
> + char i2c_path[NAME_MAX];
> + int i2c_fd;
> + int bus;
> +
> + // if no device specified, find first bus that gives us data
> + // just read one byte for now
> + for (bus = 0; bus < MAX_I2C_BUSES; bus++) {
Not sure I follow this - the bus should be specified as part of the struct
i2cm. What I would recommend is adding a i2c_fd member to the struct and
opening it as part of the probe() function.
> + if (!i2c_buses[bus])
> + continue;
> +
> + int len = snprintf(i2c_path, sizeof(i2c_path), "/dev/i2c-%d", bus);
> + if (len >= NAME_MAX)
> + continue;
> +
> + i2c_fd = 0;
> + i2c_fd = open(i2c_path, O_RDWR);
> + if (!i2c_fd)
> + continue;
> +
> +
> + if (ioctl(i2c_fd, I2C_SLAVE, addr) < 0)
For example this would become:
if (ioctl(i2cm->i2c_fd, I2C_SLAVE, addr) < 0)
And you wouldn't need the above checks or loop.
> + continue;
> +
> + res = i2c_smbus_read_byte_data(i2c_fd, offset);
> + if (res >= 0) {
> + PR_DEBUG("read %x from device %x on bus %s\n", res, addr,
i2c_path);
> + *data = (uint64_t)res;
> + return res;
> + }
> + }
> + return -1;
> +}
> +
> +static int i2cbus_probe(struct pdbg_target *target)
> +{
> + FILE *f = NULL;
> + int len;
> + char n[NAME_MAX];
> +
> + if (!target->index)
> + return -1;
> +
> + len = snprintf(n, NAME_MAX, "/dev/i2c-%i", target->index);
> + if (len >= NAME_MAX)
> + return -1;
> + f = fopen(n, "r");
> + if (f == NULL)
> + return -1;
> + i2c_buses[target->index] = 1;
Therefore you would end up with the code from the kernel_i2c_get() loop here
instead:
int len = snprintf(i2c_path, sizeof(i2c_path), "/dev/i2c-%d", bus);
if (len >= NAME_MAX)
continue;
i2c_fd = open(i2c_path, O_RDWR);
if (!i2c_fd)
return -1;
i2cm->i2c_fs = i2c_fd;
> + return 0;
> +}
> +
> +static struct i2cm i2c_bus = {
> + .target = {
> + .name = "I2C Bus",
> + .compatible = "ibm,power9-i2c-port",
> + .class = "i2cm",
> + .probe = i2cbus_probe,
> + },
> + .read = kernel_i2c_get,
> +};
> +DECLARE_HW_UNIT(i2c_bus);
> +#endif
> diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> index 4fad158..c9e6569 100644
> --- a/libpdbg/libpdbg.h
> +++ b/libpdbg/libpdbg.h
> @@ -123,6 +123,9 @@ struct pdbg_target *pdbg_address_absolute(struct
> pdbg_target *target, uint64_t * int fsi_read(struct pdbg_target *target,
> uint32_t addr, uint32_t *val); int fsi_write(struct pdbg_target *target,
> uint32_t addr, uint32_t val);
>
> +int i2c_read(struct pdbg_target *target, uint16_t port, uint32_t addr,
> + uint32_t offset, uint16_t size, uint64_t *data);
> +
> int pib_read(struct pdbg_target *target, uint64_t addr, uint64_t *val);
> int pib_write(struct pdbg_target *target, uint64_t addr, uint64_t val);
> int pib_wait(struct pdbg_target *pib_dt, uint64_t addr, uint64_t mask,
> uint64_t data); diff --git a/libpdbg/target.c b/libpdbg/target.c
> index e678470..1bf252e 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -195,6 +195,17 @@ int opb_write(struct pdbg_target *opb_dt, uint32_t
> addr, uint32_t data) return opb->write(opb, addr64, data);
> }
>
> +int i2c_read(struct pdbg_target *i2cm_dt, uint16_t port, uint32_t addr,
> uint32_t offset, uint16_t size, uint64_t *data) +{
> + struct i2cm *i2cm;
> + uint64_t addr64 = addr;
> +
> + i2cm_dt = get_class_target_addr(i2cm_dt, "i2cm", &addr64);
> + i2cm = target_to_i2cm(i2cm_dt);
> +
> + return i2cm->read(i2cm, port, addr, offset, size, data);
> +}
> +
> int fsi_read(struct pdbg_target *fsi_dt, uint32_t addr, uint32_t *data)
> {
> struct fsi *fsi;
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 04897ed..eb22377 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -138,6 +138,15 @@ struct fsi {
> };
> #define target_to_fsi(x) container_of(x, struct fsi, target)
>
> +struct i2cm {
> + struct pdbg_target target;
> + int (*read)(struct i2cm *, uint16_t, uint32_t, uint32_t, uint16_t,
> uint64_t *); + int (*write)(struct i2cm *, uint16_t, uint32_t, uint32_t,
> uint16_t, uint64_t); + bool host;
> +};
> +#define target_to_i2cm(x) container_of(x, struct i2cm, target)
> +
> +
> struct core {
> struct pdbg_target target;
> bool release_spwkup;
> diff --git a/p9-i2c.dts.m4 b/p9-i2c.dts.m4
> new file mode 100644
> index 0000000..8e8270f
> --- /dev/null
> +++ b/p9-i2c.dts.m4
> @@ -0,0 +1,21 @@
> +define(`I2CBUS', `i2c-bus@$1 {
> +bus-frequency = <0x61a80>;
> +compatible = "ibm,opal-i2c", "ibm,power8-i2c-port", "ibm,power9-i2c-port";
> +index = <$1>;
> +reg = <0x0 0x0 $1>;
> +}')dnl
> +
> +I2CBUS(0);
> +I2CBUS(1);
> +I2CBUS(2);
> +I2CBUS(3);
> +I2CBUS(4);
> +I2CBUS(5);
> +I2CBUS(6);
> +I2CBUS(7);
> +I2CBUS(8);
> +I2CBUS(9);
> +I2CBUS(10);
> +I2CBUS(11);
> +I2CBUS(12);
> +I2CBUS(13);
> diff --git a/p9-kernel.dts.m4 b/p9-kernel.dts.m4
> index 195be59..474beca 100644
> --- a/p9-kernel.dts.m4
> +++ b/p9-kernel.dts.m4
> @@ -22,6 +22,14 @@
> include(p9-pib.dts.m4)dnl
> };
>
> + i2cm at 1800 {
> + #address-cells = <0x2>;
> + #size-cells = <0x1>;
> + reg = <0x0 0x1800 0x400>;
> + compatible = "ibm,kernel-i2c-master";
> + include(p9-i2c.dts.m4)dnl
> + };
> +
> hmfsi at 100000 {
> #address-cells = <0x2>;
> #size-cells = <0x1>;
> diff --git a/src/i2c.c b/src/i2c.c
> new file mode 100644
> index 0000000..2a32ec2
> --- /dev/null
> +++ b/src/i2c.c
> @@ -0,0 +1,44 @@
> +/* Copyright 2019 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <libpdbg.h>
> +#include <inttypes.h>
> +
> +#include "main.h"
> +#include "optcmd.h"
> +#include "path.h"
> +#include "target.h"
> +
> +static int geti2c(uint16_t port, uint32_t addr, uint32_t offset, uint16_t
> size) +{
> + uint64_t data = 0xc0ffee;
> + struct pdbg_target *target, *selected = NULL;
> +
> + for_each_path_target_class("i2cbus", target) {
> + if (pdbg_target_probe(target) != PDBG_TARGET_ENABLED)
> + continue;
> + selected = target;
> + if (i2c_read(target, port, addr, offset, size, &data) == 0)
> + break;
> + break;
> + }
> +
> + if (selected == NULL)
> + return -1;
> + printf("data read: 0x%016" PRIx64 "\n", data);
> + return 0;
> +}
> +OPTCMD_DEFINE_CMD_WITH_ARGS(geti2c, geti2c, (DATA16, ADDRESS32, ADDRESS32,
> DATA16)); diff --git a/src/main.c b/src/main.c
> index d5f9385..1702efa 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -93,7 +93,7 @@ extern struct optcmd_cmd
> optcmd_threadstatus, optcmd_sreset, optcmd_regs, optcmd_probe,
> optcmd_getmem, optcmd_putmem, optcmd_getmemio, optcmd_putmemio,
> optcmd_getxer, optcmd_putxer, optcmd_getcr, optcmd_putcr,
> - optcmd_gdbserver;
> + optcmd_gdbserver, optcmd_geti2c;
>
> static struct optcmd_cmd *cmds[] = {
> &optcmd_getscom, &optcmd_putscom, &optcmd_getcfam, &optcmd_putcfam,
> @@ -103,7 +103,7 @@ static struct optcmd_cmd *cmds[] = {
> &optcmd_threadstatus, &optcmd_sreset, &optcmd_regs, &optcmd_probe,
> &optcmd_getmem, &optcmd_putmem, &optcmd_getmemio, &optcmd_putmemio,
> &optcmd_getxer, &optcmd_putxer, &optcmd_getcr, &optcmd_putcr,
> - &optcmd_gdbserver,
> + &optcmd_gdbserver, &optcmd_geti2c,
> };
>
> /* Purely for printing usage text. We could integrate printing argument and
> flag @@ -145,6 +145,7 @@ static struct action actions[] = {
> { "sreset", "", "Reset" },
> { "regs", "[--backtrace]", "State (optionally display backtrace)" },
> { "gdbserver", "", "Start a gdb server" },
> + { "geti2c", "<port> <device> <offset> <size>", "Read size bytes from the
> offset of specified device" }, };
>
> static void print_usage(void)
More information about the Pdbg
mailing list