<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    <div class="moz-cite-prefix">On 11/13/2014 01:28 PM, Wolfram Sang
      wrote:<br>
    </div>
    <blockquote cite="mid:20141113075817.GA1288@katana" type="cite">
      <pre wrap="">Hi,

I am basically fine if this goes via the powerpc-tree and I was hoping
that I could ack it right now. However, the driver looks a bit rushed
and definately needs updates before it is ready to go.

On Mon, Nov 10, 2014 at 11:35:39AM +0530, Neelesh Gupta wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:neelegup@linux.vnet.ibm.com"><neelegup@linux.vnet.ibm.com></a>
Signed-off-by: Benjamin Herrenschmidt <a class="moz-txt-link-rfc2396E" href="mailto:benh@kernel.crashing.org"><benh@kernel.crashing.org></a>
</pre>
      </blockquote>
      <pre wrap="">
Review for the I2C parts:

</pre>
      <blockquote type="cite">
        <pre wrap="">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
</pre>
      </blockquote>
      <pre wrap="">
Please sort it properly, not simply at the end.</pre>
    </blockquote>
    <br>
    I don't find match under any 'comment' in this file so keeping it
    under<br>
    "Other I2C/SMBus bus drivers".<br>
    <br>
    <blockquote cite="mid:20141113075817.GA1288@katana" type="cite">
      <pre wrap="">

</pre>
      <blockquote type="cite">
        <pre wrap=""> 
 ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
new file mode 100644
index 0000000..3261716
--- /dev/null
+++ b/drivers/i2c/busses/i2c-opal.c
+
+       return rc;
+}
+
+static int i2c_opal_remove(struct platform_device *pdev)
+{
+       struct i2c_adapter *adapter = platform_get_drvdata(pdev);
+
+       i2c_del_adapter(adapter);
+
+       kfree(adapter);
+
+       return 0;
+}
+
+static const struct of_device_id i2c_opal_of_match[] = {
+       {
+               .compatible = "ibm,power8-i2c-port",
+       },
+       { }
+};
+MODULE_DEVICE_TABLE(of, i2c_opal_of_match);
+
+static struct platform_driver i2c_opal_driver = {
+       .probe  = i2c_opal_probe,
+       .remove = i2c_opal_remove,
+       .driver = {
+               .name           = "i2c-opal",
+               .owner          = THIS_MODULE,
</pre>
      </blockquote>
      <pre wrap="">
Not needed.</pre>
    </blockquote>
    <br>
    I didn't get this.<br>
    <br>
    <blockquote cite="mid:20141113075817.GA1288@katana" type="cite">
      <pre wrap="">

</pre>
      <blockquote type="cite">
        <pre wrap="">+          .of_match_table = i2c_opal_of_match,
+       },
+};
+
+static int __init i2c_opal_init(void)
+{
+       if (!firmware_has_feature(FW_FEATURE_OPAL))
+               return -ENODEV;
+
+       return platform_driver_register(&i2c_opal_driver);
+}
+
+static void __exit i2c_opal_exit(void)
+{
+       return platform_driver_unregister(&i2c_opal_driver);
+}
+
+MODULE_AUTHOR("Neelesh Gupta <a class="moz-txt-link-rfc2396E" href="mailto:neelegup@linux.vnet.ibm.com"><neelegup@linux.vnet.ibm.com></a>");
+MODULE_DESCRIPTION("IBM OPAL I2C driver");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_opal_init);
+module_exit(i2c_opal_exit);
</pre>
      </blockquote>
      <pre wrap="">
Please put thos right below the functions it references.</pre>
    </blockquote>
    <br>
    Okay.<br>
    <br>
    - Neelesh<br>
    <blockquote cite="mid:20141113075817.GA1288@katana" type="cite">
      <pre wrap="">

</pre>
      <blockquote type="cite">
        <pre wrap="">
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to <a class="moz-txt-link-abbreviated" href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a>
More majordomo info at  <a class="moz-txt-link-freetext" href="http://vger.kernel.org/majordomo-info.html">http://vger.kernel.org/majordomo-info.html</a>
</pre>
        <br>
        <fieldset class="mimeAttachmentHeader"></fieldset>
        <br>
        <pre wrap="">_______________________________________________
Linuxppc-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:Linuxppc-dev@lists.ozlabs.org">Linuxppc-dev@lists.ozlabs.org</a>
<a class="moz-txt-link-freetext" href="https://lists.ozlabs.org/listinfo/linuxppc-dev">https://lists.ozlabs.org/listinfo/linuxppc-dev</a></pre>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>