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