On Sun, Nov 16, 2014 at 10:47:46PM +0530, Neelesh Gupta wrote:
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
> The driver uses the platform device infrastructure to probe the busses
> on the platform and registers them with the i2c driver framework.
>
> Signed-off-by: Neelesh Gupta <neelegup@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
...
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 917c358..71ad6e1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -1044,4 +1044,15 @@ config SCx200_ACB
> This support is also available as a module. If so, the module
> will be called scx200_acb.
>
> +config I2C_OPAL
> + tristate "IBM OPAL I2C driver"
> + depends on PPC_POWERNV
> + default y
> + help
> + This exposes the PowerNV platform i2c busses to the linux i2c layer,
> + the driver is based on the OPAL interfaces.
> +
> + This driver can also be built as a module. If so, the module will be
> + called as i2c-opal.
> +
> endmenu
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 78d56c5..350aa86 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -102,5 +102,6 @@ obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
> obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
> obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
> obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
> +obj-$(CONFIG_I2C_OPAL) += i2c-opal.o
Please keep it proprly sorted.
> + rc = of_property_read_u32(pdev->dev.of_node, "ibm,opal-id", &opal_id);
> + if (rc) {
> + dev_err(&pdev->dev, "Missing ibm,opal-id property !\n");
> + return -EIO;
> + }
You introduce new bindings which need to be documented in
Docuemntation/devicetree/bindings/i2c.
They should be posted as a seperate patch with
devicetree@xxxxxxxxxxxxxxx CCed, so they can comment on it. This is
required these days and especially important..
> + adapter = devm_kzalloc(&pdev->dev, sizeof(*adapter), GFP_KERNEL);
> + if (!adapter)
> + return -ENOMEM;
> +
> + adapter->algo = &i2c_opal_algo;
> + adapter->algo_data = (void *)(unsigned long)opal_id;
> + adapter->dev.parent = &pdev->dev;
> + adapter->dev.of_node = of_node_get(pdev->dev.of_node);
> + pname = of_get_property(pdev->dev.of_node, "ibm,port-name", NULL);
> + if (pname)
> + strlcpy(adapter->name, pname, sizeof(adapter->name));
> + else
> + strlcpy(adapter->name, "opal", sizeof(adapter->name));
... because I'd like to get an ack from them because of this binding. I
don't know if we can just say "this comes from firmware, so we must
support it" (although you wrote the firmware IIUC) or if we have to
judge if this is a HW description which should go into DT? I am open
meanwhile that the adapter name does not need to be static anymore.
However, I don't know much about server world and FW, so maybe they can
assist.
An example binding in that document would also be very helpful.
> +static struct platform_driver i2c_opal_driver = {
> + .probe = i2c_opal_probe,
> + .remove = i2c_opal_remove,
> + .driver = {
> + .name = "i2c-opal",
> + .owner = THIS_MODULE,
owner not needed.
Thanks,
Wolfram
Attachment:
signature.asc
Description: Digital signature
