Re: [U-Boot] [PATCH 2/2] devinfo: Add gazerbeam info driver

[Mario Six]

Hi Simon,

On Fri, Mar 30, 2018 at 10:40 AM, Simon Glass  wrote:
> Hi Mario,
>
> On 28 March 2018 at 20:36, Mario Six  wrote:
>> Add a device information driver for the upcoming gdsys Gazerbeam board.
>>
>> Signed-off-by: Mario Six 
>> ---
>>  drivers/devinfo/gazerbeam.c | 151 
>> 
>>  drivers/devinfo/gazerbeam.h |  17 +
>>  2 files changed, 168 insertions(+)
>>  create mode 100644 drivers/devinfo/gazerbeam.c
>>  create mode 100644 drivers/devinfo/gazerbeam.h
>>
>> diff --git a/drivers/devinfo/gazerbeam.c b/drivers/devinfo/gazerbeam.c
>> new file mode 100644
>> index 00..10226aa254
>> --- /dev/null
>> +++ b/drivers/devinfo/gazerbeam.c
>> @@ -0,0 +1,151 @@
>> +/*
>> + * (C) Copyright 2017
>> + * Mario Six,  Guntermann & Drunck GmbH, mario@gdsys.cc
>> + *
>> + * SPDX-License-Identifier:GPL-2.0+
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "gazerbeam.h"
>> +
>> +const int VER_GPIOS_COUNT = 4;
>> +
>> +struct devinfo_gazerbeam_priv {
>> +   struct gpio_desc var_gpios[2];
>> +   struct gpio_desc ver_gpios[4];
>> +   int variant;
>> +   int multichannel;
>> +   int hwversion;
>> +};
>> +
>> +static int _read_multichannel_variant(struct udevice *dev)
>> +{
>> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
>> +   struct udevice *i2c_bus;
>> +   struct udevice *dummy;
>> +   char *listname;
>> +   bool mc4, mc2, sc, con;
>> +
>> +   uclass_get_device_by_seq(UCLASS_I2C, 1, _bus);
>
> Please pass the error through unless there is a good reason not to.
>
> ret = uclass ...
> if (ret) {
>debug(...)
>return ret
> }
>
>> +
>> +   if (!i2c_bus) {
>> +   puts("Could not get I2C bus\n");
>> +   return -EIO;
>> +   }
>> +
>> +   mc2 = !dm_i2c_probe(i2c_bus, 0x20, 0, );
>> +   mc4 = !dm_i2c_probe(i2c_bus, 0x22, 0, );
>
> Same here
>> +
>> +   if (mc2 && mc4) {
>> +   puts("Board hardware configuration inconsistent.\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   listname = mc2 ? "var_gpios_mc2" : "var_gpios_mc4";
>
> If these are DT properties they should use hyphen instead of underscore
>
>> +
>> +   if (!gpio_request_list_by_name(dev, listname, priv->var_gpios,
>> +  2, GPIOD_IS_IN)) {
>> +   printf("Requesting gpio list %s failed.\n", listname);
>> +   return -ENODEV;
>
> Return correct error
>
>> +   }
>> +
>> +   sc = dm_gpio_get_value(>var_gpios[0]);
>> +   con = dm_gpio_get_value(>var_gpios[1]);
>
> And here
>> +
>> +   if ((sc && mc2) || (sc && mc4) || (!sc && !mc2 && !mc4)) {
>> +   puts("Board hardware configuration inconsistent.\n");
>> +   return -EINVAL;
>> +   }
>> +
>> +   priv->variant = con ? VAR_CON : VAR_CPU;
>> +
>> +   priv->multichannel = mc4 ? 4 : (mc2 ? 2 : (sc ? 1 : 0));
>> +
>> +   return 0;
>> +}
>> +
>> +static int _read_hwversion(struct udevice *dev)
>> +{
>> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
>> +   int value;
>> +
>> +   if (!gpio_request_list_by_name(dev, "ver_gpios", priv->ver_gpios,
>
> ver-gpios ?
>
>> +  VER_GPIOS_COUNT, GPIOD_IS_IN))
>> +   return -ENODEV;
>> +
>> +   value = dm_gpio_get_values_as_int(priv->ver_gpios, VER_GPIOS_COUNT);
>> +
>> +   if (value < 0)
>> +   return value;
>> +
>> +   priv->hwversion = value;
>> +
>> +   gpio_free_list(dev, priv->ver_gpios, VER_GPIOS_COUNT);
>> +
>> +   return 0;
>> +}
>> +
>> +int devinfo_gazerbeam_detect(struct udevice *dev)
>> +{
>> +   int res;
>> +
>> +   res = _read_multichannel_variant(dev);
>> +   if (res)
>> +   return res;
>> +
>> +   res = _read_hwversion(dev);
>> +   if (res)
>> +   return res;
>> +
>> +   return 0;
>> +}
>> +
>> +int devinfo_gazerbeam_get_int(struct udevice *dev, int id, int *val)
>> +{
>> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
>> +
>> +   switch (id) {
>> +   case DEVINFO_MULTICHANNEL:
>> +   *val = priv->multichannel;
>> +   break;
>> +   case DEVINFO_VARIANT:
>> +   *val = priv->variant;
>> +   break;
>> +   case DEVINFO_HWVERSION:
>> +   *val = priv->hwversion;
>> +   break;
>> +   default:
>> +   return -EINVAL;
>> +   }
>> +
>> +   return 0;
>> +}
>> +
>> +static const struct udevice_id devinfo_gazerbeam_ids[] = {
>> +   { .compatible = "gdsys,devinfo_gazerbeam" },
>> +   { /* sentinel */ }
>> +};
>> +
>> +static const struct devinfo_ops devinfo_gazerbeam_ops = {
>> +   .detect = devinfo_gazerbeam_detect,
>> +   .get_int = devinfo_gazerbeam_get_int,
>> +};
>> +

Re: [U-Boot] [PATCH 2/2] devinfo: Add gazerbeam info driver

[Simon Glass]

Hi Mario,

On 28 March 2018 at 20:36, Mario Six  wrote:
> Add a device information driver for the upcoming gdsys Gazerbeam board.
>
> Signed-off-by: Mario Six 
> ---
>  drivers/devinfo/gazerbeam.c | 151 
> 
>  drivers/devinfo/gazerbeam.h |  17 +
>  2 files changed, 168 insertions(+)
>  create mode 100644 drivers/devinfo/gazerbeam.c
>  create mode 100644 drivers/devinfo/gazerbeam.h
>
> diff --git a/drivers/devinfo/gazerbeam.c b/drivers/devinfo/gazerbeam.c
> new file mode 100644
> index 00..10226aa254
> --- /dev/null
> +++ b/drivers/devinfo/gazerbeam.c
> @@ -0,0 +1,151 @@
> +/*
> + * (C) Copyright 2017
> + * Mario Six,  Guntermann & Drunck GmbH, mario@gdsys.cc
> + *
> + * SPDX-License-Identifier:GPL-2.0+
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "gazerbeam.h"
> +
> +const int VER_GPIOS_COUNT = 4;
> +
> +struct devinfo_gazerbeam_priv {
> +   struct gpio_desc var_gpios[2];
> +   struct gpio_desc ver_gpios[4];
> +   int variant;
> +   int multichannel;
> +   int hwversion;
> +};
> +
> +static int _read_multichannel_variant(struct udevice *dev)
> +{
> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
> +   struct udevice *i2c_bus;
> +   struct udevice *dummy;
> +   char *listname;
> +   bool mc4, mc2, sc, con;
> +
> +   uclass_get_device_by_seq(UCLASS_I2C, 1, _bus);

Please pass the error through unless there is a good reason not to.

ret = uclass ...
if (ret) {
   debug(...)
   return ret
}

> +
> +   if (!i2c_bus) {
> +   puts("Could not get I2C bus\n");
> +   return -EIO;
> +   }
> +
> +   mc2 = !dm_i2c_probe(i2c_bus, 0x20, 0, );
> +   mc4 = !dm_i2c_probe(i2c_bus, 0x22, 0, );

Same here
> +
> +   if (mc2 && mc4) {
> +   puts("Board hardware configuration inconsistent.\n");
> +   return -EINVAL;
> +   }
> +
> +   listname = mc2 ? "var_gpios_mc2" : "var_gpios_mc4";

If these are DT properties they should use hyphen instead of underscore

> +
> +   if (!gpio_request_list_by_name(dev, listname, priv->var_gpios,
> +  2, GPIOD_IS_IN)) {
> +   printf("Requesting gpio list %s failed.\n", listname);
> +   return -ENODEV;

Return correct error

> +   }
> +
> +   sc = dm_gpio_get_value(>var_gpios[0]);
> +   con = dm_gpio_get_value(>var_gpios[1]);

And here
> +
> +   if ((sc && mc2) || (sc && mc4) || (!sc && !mc2 && !mc4)) {
> +   puts("Board hardware configuration inconsistent.\n");
> +   return -EINVAL;
> +   }
> +
> +   priv->variant = con ? VAR_CON : VAR_CPU;
> +
> +   priv->multichannel = mc4 ? 4 : (mc2 ? 2 : (sc ? 1 : 0));
> +
> +   return 0;
> +}
> +
> +static int _read_hwversion(struct udevice *dev)
> +{
> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
> +   int value;
> +
> +   if (!gpio_request_list_by_name(dev, "ver_gpios", priv->ver_gpios,

ver-gpios ?

> +  VER_GPIOS_COUNT, GPIOD_IS_IN))
> +   return -ENODEV;
> +
> +   value = dm_gpio_get_values_as_int(priv->ver_gpios, VER_GPIOS_COUNT);
> +
> +   if (value < 0)
> +   return value;
> +
> +   priv->hwversion = value;
> +
> +   gpio_free_list(dev, priv->ver_gpios, VER_GPIOS_COUNT);
> +
> +   return 0;
> +}
> +
> +int devinfo_gazerbeam_detect(struct udevice *dev)
> +{
> +   int res;
> +
> +   res = _read_multichannel_variant(dev);
> +   if (res)
> +   return res;
> +
> +   res = _read_hwversion(dev);
> +   if (res)
> +   return res;
> +
> +   return 0;
> +}
> +
> +int devinfo_gazerbeam_get_int(struct udevice *dev, int id, int *val)
> +{
> +   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
> +
> +   switch (id) {
> +   case DEVINFO_MULTICHANNEL:
> +   *val = priv->multichannel;
> +   break;
> +   case DEVINFO_VARIANT:
> +   *val = priv->variant;
> +   break;
> +   case DEVINFO_HWVERSION:
> +   *val = priv->hwversion;
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> +
> +   return 0;
> +}
> +
> +static const struct udevice_id devinfo_gazerbeam_ids[] = {
> +   { .compatible = "gdsys,devinfo_gazerbeam" },
> +   { /* sentinel */ }
> +};
> +
> +static const struct devinfo_ops devinfo_gazerbeam_ops = {
> +   .detect = devinfo_gazerbeam_detect,
> +   .get_int = devinfo_gazerbeam_get_int,
> +};
> +
> +int devinfo_gazerbeam_probe(struct udevice *dev)
> +{
> +   return 0;
> +}
> +
> +U_BOOT_DRIVER(devinfo_gazerbeam) = {
> +   .name   = "devinfo_gazerbeam",
> +   .id = UCLASS_DEVINFO,
> +   .of_match   = devinfo_gazerbeam_ids,
> +  

[U-Boot] [PATCH 2/2] devinfo: Add gazerbeam info driver

[Mario Six]

Add a device information driver for the upcoming gdsys Gazerbeam board.

Signed-off-by: Mario Six 
---
 drivers/devinfo/gazerbeam.c | 151 
 drivers/devinfo/gazerbeam.h |  17 +
 2 files changed, 168 insertions(+)
 create mode 100644 drivers/devinfo/gazerbeam.c
 create mode 100644 drivers/devinfo/gazerbeam.h

diff --git a/drivers/devinfo/gazerbeam.c b/drivers/devinfo/gazerbeam.c
new file mode 100644
index 00..10226aa254
--- /dev/null
+++ b/drivers/devinfo/gazerbeam.c
@@ -0,0 +1,151 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario@gdsys.cc
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "gazerbeam.h"
+
+const int VER_GPIOS_COUNT = 4;
+
+struct devinfo_gazerbeam_priv {
+   struct gpio_desc var_gpios[2];
+   struct gpio_desc ver_gpios[4];
+   int variant;
+   int multichannel;
+   int hwversion;
+};
+
+static int _read_multichannel_variant(struct udevice *dev)
+{
+   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
+   struct udevice *i2c_bus;
+   struct udevice *dummy;
+   char *listname;
+   bool mc4, mc2, sc, con;
+
+   uclass_get_device_by_seq(UCLASS_I2C, 1, _bus);
+
+   if (!i2c_bus) {
+   puts("Could not get I2C bus\n");
+   return -EIO;
+   }
+
+   mc2 = !dm_i2c_probe(i2c_bus, 0x20, 0, );
+   mc4 = !dm_i2c_probe(i2c_bus, 0x22, 0, );
+
+   if (mc2 && mc4) {
+   puts("Board hardware configuration inconsistent.\n");
+   return -EINVAL;
+   }
+
+   listname = mc2 ? "var_gpios_mc2" : "var_gpios_mc4";
+
+   if (!gpio_request_list_by_name(dev, listname, priv->var_gpios,
+  2, GPIOD_IS_IN)) {
+   printf("Requesting gpio list %s failed.\n", listname);
+   return -ENODEV;
+   }
+
+   sc = dm_gpio_get_value(>var_gpios[0]);
+   con = dm_gpio_get_value(>var_gpios[1]);
+
+   if ((sc && mc2) || (sc && mc4) || (!sc && !mc2 && !mc4)) {
+   puts("Board hardware configuration inconsistent.\n");
+   return -EINVAL;
+   }
+
+   priv->variant = con ? VAR_CON : VAR_CPU;
+
+   priv->multichannel = mc4 ? 4 : (mc2 ? 2 : (sc ? 1 : 0));
+
+   return 0;
+}
+
+static int _read_hwversion(struct udevice *dev)
+{
+   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
+   int value;
+
+   if (!gpio_request_list_by_name(dev, "ver_gpios", priv->ver_gpios,
+  VER_GPIOS_COUNT, GPIOD_IS_IN))
+   return -ENODEV;
+
+   value = dm_gpio_get_values_as_int(priv->ver_gpios, VER_GPIOS_COUNT);
+
+   if (value < 0)
+   return value;
+
+   priv->hwversion = value;
+
+   gpio_free_list(dev, priv->ver_gpios, VER_GPIOS_COUNT);
+
+   return 0;
+}
+
+int devinfo_gazerbeam_detect(struct udevice *dev)
+{
+   int res;
+
+   res = _read_multichannel_variant(dev);
+   if (res)
+   return res;
+
+   res = _read_hwversion(dev);
+   if (res)
+   return res;
+
+   return 0;
+}
+
+int devinfo_gazerbeam_get_int(struct udevice *dev, int id, int *val)
+{
+   struct devinfo_gazerbeam_priv *priv = dev_get_priv(dev);
+
+   switch (id) {
+   case DEVINFO_MULTICHANNEL:
+   *val = priv->multichannel;
+   break;
+   case DEVINFO_VARIANT:
+   *val = priv->variant;
+   break;
+   case DEVINFO_HWVERSION:
+   *val = priv->hwversion;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static const struct udevice_id devinfo_gazerbeam_ids[] = {
+   { .compatible = "gdsys,devinfo_gazerbeam" },
+   { /* sentinel */ }
+};
+
+static const struct devinfo_ops devinfo_gazerbeam_ops = {
+   .detect = devinfo_gazerbeam_detect,
+   .get_int = devinfo_gazerbeam_get_int,
+};
+
+int devinfo_gazerbeam_probe(struct udevice *dev)
+{
+   return 0;
+}
+
+U_BOOT_DRIVER(devinfo_gazerbeam) = {
+   .name   = "devinfo_gazerbeam",
+   .id = UCLASS_DEVINFO,
+   .of_match   = devinfo_gazerbeam_ids,
+   .ops= _gazerbeam_ops,
+   .priv_auto_alloc_size = sizeof(struct devinfo_gazerbeam_priv),
+   .probe  = devinfo_gazerbeam_probe,
+};
diff --git a/drivers/devinfo/gazerbeam.h b/drivers/devinfo/gazerbeam.h
new file mode 100644
index 00..534364416c
--- /dev/null
+++ b/drivers/devinfo/gazerbeam.h
@@ -0,0 +1,17 @@
+/*
+ * (C) Copyright 2017
+ * Mario Six,  Guntermann & Drunck GmbH, mario@gdsys.cc
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+enum {
+   DEVINFO_MULTICHANNEL,
+   DEVINFO_VARIANT,
+   DEVINFO_HWVERSION,
+};
+
+enum {
+   VAR_CON,
+   VAR_CPU,
+};
-- 
2.16.1