From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neelesh Gupta Subject: Re: [PATCH] i2c: Driver to expose PowerNV platform i2c busses Date: Fri, 14 Nov 2014 21:40:02 +0530 Message-ID: <5466295A.1040005@linux.vnet.ibm.com> References: <20141110060424.9407.2498.stgit@localhost.localdomain> <20141113075817.GA1288@katana> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3376975262167897017==" Return-path: In-Reply-To: <20141113075817.GA1288@katana> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Wolfram Sang Cc: linuxppc-dev@ozlabs.org, linux-i2c@vger.kernel.org List-Id: linux-i2c@vger.kernel.org This is a multi-part message in MIME format. --===============3376975262167897017== Content-Type: multipart/alternative; boundary="------------040702070404030300020900" This is a multi-part message in MIME format. --------------040702070404030300020900 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/13/2014 01:28 PM, Wolfram Sang wrote: > 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: >> 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 >> Signed-off-by: Benjamin Herrenschmidt > Review for the I2C parts: > >> 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 sort it properly, not simply at the end. I don't find match under any 'comment' in this file so keeping it under "Other I2C/SMBus bus drivers". > >> >> 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, > Not needed. I didn't get this. > >> + .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 "); >> +MODULE_DESCRIPTION("IBM OPAL I2C driver"); >> +MODULE_LICENSE("GPL"); >> + >> +module_init(i2c_opal_init); >> +module_exit(i2c_opal_exit); > Please put thos right below the functions it references. Okay. - Neelesh > >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> _______________________________________________ >> Linuxppc-dev mailing list >> Linuxppc-dev@lists.ozlabs.org >> https://lists.ozlabs.org/listinfo/linuxppc-dev --------------040702070404030300020900 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
On 11/13/2014 01:28 PM, Wolfram Sang wrote:
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:

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@linux.vnet.ibm.com>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Review for the I2C parts:


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 sort it properly, not simply at the end.

I don't find match under any 'comment' in this file so keeping it under
"Other I2C/SMBus bus drivers".



 
 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,

Not needed.

I didn't get this.



+		.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 <neelegup@linux.vnet.ibm.com>");
+MODULE_DESCRIPTION("IBM OPAL I2C driver");
+MODULE_LICENSE("GPL");
+
+module_init(i2c_opal_init);
+module_exit(i2c_opal_exit);

Please put thos right below the functions it references.

Okay.

- Neelesh


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

--------------040702070404030300020900-- --===============3376975262167897017== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXhwcGMt ZGV2IG1haWxpbmcgbGlzdApMaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xp c3RzLm96bGFicy5vcmcvbGlzdGluZm8vbGludXhwcGMtZGV2 --===============3376975262167897017==--