Subject: [PATCH v1 5/6] eeprom: qfprom: Add Qualcomm QFPROM support.

From: [email-masked] (Srinivas Kandagatla)
Date: Thu, 05 Mar 2015 10:10:48 +0000

Thankyou for the comments,

On 05/03/15 10:02, Paul Bolle wrote:
> On Thu, 2015-03-05 at 09:46 +0000, Srinivas Kandagatla wrote:
>> diff --git a/drivers/eeprom/Kconfig b/drivers/eeprom/Kconfig
>> index bff8ecb..65325c7 100644
>> --- a/drivers/eeprom/Kconfig
>> +++ b/drivers/eeprom/Kconfig
>> @@ -28,4 +28,11 @@ config EEPROM_SUNXI_SID
>>   	  This driver can also be built as a module. If so, the module
>>   	  will be called sunxi_sid.
>>
>> +config QCOM_QFPROM
>> +	tristate "QCOM QFPROM Support"
>> +        depends on EEPROM
>
> Make this one tab, please.
>
>> +	select REGMAP_MMIO
>> +	help
>> +          Say y here to enable QFPROM support. The QFPROM provides access
>> +          functions for QFPROM data to rest of the drivers via eeprom interface.
>
> And this one tab and two spaces, please.
>
> All utterly trivial, of course, but I found a less trivial problem with
> this patch, so I included these two comments anyway.
>
Will fix it in next version.
>>   endif
>> diff --git a/drivers/eeprom/Makefile b/drivers/eeprom/Makefile
>> index 661422c..f99c824 100644
>> --- a/drivers/eeprom/Makefile
>> +++ b/drivers/eeprom/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_EEPROM)		+= core.o
>>
>>   # Devices
>>   obj-$(CONFIG_EEPROM_SUNXI_SID)	+= eeprom-sunxi-sid.o
>> +obj-$(CONFIG_QCOM_QFPROM)	+= qfprom.o
>> diff --git a/drivers/eeprom/qfprom.c b/drivers/eeprom/qfprom.c
>> new file mode 100644
>> index 0000000..371a8c0
>> --- /dev/null
>> +++ b/drivers/eeprom/qfprom.c
>> @@ -0,0 +1,74 @@
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/eeprom-provider.h>
>
> [...]
>
>> +MODULE_LICENSE("GPL2");
>
> This will taint the kernel on module load. I guess you meant
>      MODULE_LICENSE("GPL v2");
>
> but there's no comment with some lines about the license at the top of
> this file, so I can't be sure.
>
Yes, I meant GPL v2, and will fix the license header too in next version.

--srini

>
> Paul Bolle
>
> (Chances are that by the end of this week everybody is so tired of
> messages like this that people actually check this stuff before
> submitting, and there's no need to review this anymore for the rest of
> this year. That would be mission accomplished, I guess.)
>