diff mbox

[1/5] rtc: m48t86: verify that the RTC is actually present

Message ID 20170215163527.5697-2-hsweeten@visionengravers.com
State Accepted
Headers show

Commit Message

H Hartley Sweeten Feb. 15, 2017, 4:35 p.m. UTC
The RTC is an optional feature at purchase time on some Technologic
Systems boards. Verify that it actually exists by checking if the
last two bytes of the NVRAM can be changed.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Alexander Clouter <alex@digriz.org.uk>
---
 drivers/rtc/rtc-m48t86.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Alexandre Belloni Feb. 21, 2017, 4:37 p.m. UTC | #1
Hi,

On 15/02/2017 at 09:35:23 -0700, H Hartley Sweeten wrote:
> The RTC is an optional feature at purchase time on some Technologic
> Systems boards. Verify that it actually exists by checking if the
> last two bytes of the NVRAM can be changed.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Alexander Clouter <alex@digriz.org.uk>
> ---
>  drivers/rtc/rtc-m48t86.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 

I've applied that one. Alex, if you are fine with the mach-orion5x
changes, I can take them for 4.11. Else, this will wait for 4.12.

> diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
> index 4dc4af4..41ac5d6 100644
> --- a/drivers/rtc/rtc-m48t86.c
> +++ b/drivers/rtc/rtc-m48t86.c
> @@ -201,6 +201,37 @@ static ssize_t m48t86_nvram_write(struct file *filp, struct kobject *kobj,
>  static BIN_ATTR(nvram, 0644, m48t86_nvram_read, m48t86_nvram_write,
>  		M48T86_NVRAM_LEN);
>  
> +/*
> + * The RTC is an optional feature at purchase time on some Technologic Systems
> + * boards. Verify that it actually exists by checking if the last two bytes
> + * of the NVRAM can be changed.
> + *
> + * This is based on the method used in their rtc7800.c example.
> + */
> +static bool m48t86_verify_chip(struct platform_device *pdev)
> +{
> +	unsigned int offset0 = M48T86_NVRAM_LEN - 2;
> +	unsigned int offset1 = M48T86_NVRAM_LEN - 1;
> +	unsigned char tmp0, tmp1;
> +
> +	tmp0 = m48t86_readb(&pdev->dev, offset0);
> +	tmp1 = m48t86_readb(&pdev->dev, offset1);
> +
> +	m48t86_writeb(&pdev->dev, 0x00, offset0);
> +	m48t86_writeb(&pdev->dev, 0x55, offset1);
> +	if (m48t86_readb(&pdev->dev, offset1) == 0x55) {
> +		m48t86_writeb(&pdev->dev, 0xaa, offset1);
> +		if (m48t86_readb(&pdev->dev, offset1) == 0xaa &&
> +		    m48t86_readb(&pdev->dev, offset0) == 0x00) {
> +			m48t86_writeb(&pdev->dev, tmp0, offset0);
> +			m48t86_writeb(&pdev->dev, tmp1, offset1);
> +
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> +
>  static int m48t86_rtc_probe(struct platform_device *pdev)
>  {
>  	struct m48t86_rtc_info *info;
> @@ -231,6 +262,11 @@ static int m48t86_rtc_probe(struct platform_device *pdev)
>  
>  	dev_set_drvdata(&pdev->dev, info);
>  
> +	if (!m48t86_verify_chip(pdev)) {
> +		dev_info(&pdev->dev, "RTC not present\n");
> +		return -ENODEV;
> +	}
> +
>  	info->rtc = devm_rtc_device_register(&pdev->dev, "m48t86",
>  					     &m48t86_rtc_ops, THIS_MODULE);
>  	if (IS_ERR(info->rtc))
> -- 
> 2.10.0
>
Hartley Sweeten Feb. 21, 2017, 5:04 p.m. UTC | #2
On Tuesday, February 21, 2017 9:37 AM, Alexandre Belloni wrote:
> On 15/02/2017 at 09:35:23 -0700, H Hartley Sweeten wrote:
>> The RTC is an optional feature at purchase time on some Technologic
>> Systems boards. Verify that it actually exists by checking if the
>> last two bytes of the NVRAM can be changed.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Cc: Alexander Clouter <alex@digriz.org.uk>
>> ---
>>  drivers/rtc/rtc-m48t86.c | 36 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>> 
>
> I've applied that one. Alex, if you are fine with the mach-orion5x
> changes, I can take them for 4.11. Else, this will wait for 4.12.

Hello,

Thanks. The change to mach-ep93xx is ok if you want to take that one.

I did just notice a minor issue with this one. It will still work as-is but
It's not technically doing what was done in mach-orion52. See below...

>> diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
>> index 4dc4af4..41ac5d6 100644
>> --- a/drivers/rtc/rtc-m48t86.c
>> +++ b/drivers/rtc/rtc-m48t86.c
>> @@ -201,6 +201,37 @@ static ssize_t m48t86_nvram_write(struct file *filp, struct kobject *kobj,
>>  static BIN_ATTR(nvram, 0644, m48t86_nvram_read, m48t86_nvram_write,
>>  		M48T86_NVRAM_LEN);
>>  
>> +/*
>> + * The RTC is an optional feature at purchase time on some Technologic Systems
>> + * boards. Verify that it actually exists by checking if the last two bytes
>> + * of the NVRAM can be changed.
>> + *
>> + * This is based on the method used in their rtc7800.c example.
>> + */
>> +static bool m48t86_verify_chip(struct platform_device *pdev)
>> +{
>> +	unsigned int offset0 = M48T86_NVRAM_LEN - 2;
>> +	unsigned int offset1 = M48T86_NVRAM_LEN - 1;

In order to address the last two bytes, these two offsets should actually be:

+	unsigned int offset0 = M48T86_NVRAM(M48T86_NVRAM_LEN - 2);
+	unsigned int offset1 = M48T86_NVRAM(M48T86_NVRAM_LEN - 1);

As they are right now, they are both off by 14 bytes (0x0e).

I can send a patch to fix it or you can do it if you prefer.

Sorry about that.
Hartley
Alexandre Belloni Feb. 21, 2017, 5:21 p.m. UTC | #3
On 21/02/2017 at 17:04:07 +0000, Hartley Sweeten wrote:
> On Tuesday, February 21, 2017 9:37 AM, Alexandre Belloni wrote:
> > On 15/02/2017 at 09:35:23 -0700, H Hartley Sweeten wrote:
> >> The RTC is an optional feature at purchase time on some Technologic
> >> Systems boards. Verify that it actually exists by checking if the
> >> last two bytes of the NVRAM can be changed.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >> Cc: Alessandro Zummo <a.zummo@towertech.it>
> >> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >> Cc: Alexander Clouter <alex@digriz.org.uk>
> >> ---
> >>  drivers/rtc/rtc-m48t86.c | 36 ++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 36 insertions(+)
> >> 
> >
> > I've applied that one. Alex, if you are fine with the mach-orion5x
> > changes, I can take them for 4.11. Else, this will wait for 4.12.
> 
> Hello,
> 
> Thanks. The change to mach-ep93xx is ok if you want to take that one.
> 

Ok, I took it.

> In order to address the last two bytes, these two offsets should actually be:
> 
> +	unsigned int offset0 = M48T86_NVRAM(M48T86_NVRAM_LEN - 2);
> +	unsigned int offset1 = M48T86_NVRAM(M48T86_NVRAM_LEN - 1);
> 
> As they are right now, they are both off by 14 bytes (0x0e).
> 
> I can send a patch to fix it or you can do it if you prefer.
> 

I fixed it up directly in rtc-next
Alexander Clouter Feb. 21, 2017, 5:47 p.m. UTC | #4
Thanks, not tested though, as I am lacking in time at the moment.  I 
probably am the only ts7800 user in the world using a mainline kernel 
too...so will only be punishing myself :-)

Acked-By: Alexander Clouter <alex+kernel@digriz.org.uk>


On 21 February 2017 4:37:35 p.m. Alexandre Belloni 
<alexandre.belloni@free-electrons.com> wrote:

> Hi,
>
> On 15/02/2017 at 09:35:23 -0700, H Hartley Sweeten wrote:
>> The RTC is an optional feature at purchase time on some Technologic
>> Systems boards. Verify that it actually exists by checking if the
>> last two bytes of the NVRAM can be changed.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> Cc: Alexander Clouter <alex@digriz.org.uk>
>> ---
>>  drivers/rtc/rtc-m48t86.c | 36 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>
> I've applied that one. Alex, if you are fine with the mach-orion5x
> changes, I can take them for 4.11. Else, this will wait for 4.12.
>
>> diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
>> index 4dc4af4..41ac5d6 100644
>> --- a/drivers/rtc/rtc-m48t86.c
>> +++ b/drivers/rtc/rtc-m48t86.c
>> @@ -201,6 +201,37 @@ static ssize_t m48t86_nvram_write(struct file *filp, 
>> struct kobject *kobj,
>>  static BIN_ATTR(nvram, 0644, m48t86_nvram_read, m48t86_nvram_write,
>>  		M48T86_NVRAM_LEN);
>>
>> +/*
>> + * The RTC is an optional feature at purchase time on some Technologic Systems
>> + * boards. Verify that it actually exists by checking if the last two bytes
>> + * of the NVRAM can be changed.
>> + *
>> + * This is based on the method used in their rtc7800.c example.
>> + */
>> +static bool m48t86_verify_chip(struct platform_device *pdev)
>> +{
>> +	unsigned int offset0 = M48T86_NVRAM_LEN - 2;
>> +	unsigned int offset1 = M48T86_NVRAM_LEN - 1;
>> +	unsigned char tmp0, tmp1;
>> +
>> +	tmp0 = m48t86_readb(&pdev->dev, offset0);
>> +	tmp1 = m48t86_readb(&pdev->dev, offset1);
>> +
>> +	m48t86_writeb(&pdev->dev, 0x00, offset0);
>> +	m48t86_writeb(&pdev->dev, 0x55, offset1);
>> +	if (m48t86_readb(&pdev->dev, offset1) == 0x55) {
>> +		m48t86_writeb(&pdev->dev, 0xaa, offset1);
>> +		if (m48t86_readb(&pdev->dev, offset1) == 0xaa &&
>> +		    m48t86_readb(&pdev->dev, offset0) == 0x00) {
>> +			m48t86_writeb(&pdev->dev, tmp0, offset0);
>> +			m48t86_writeb(&pdev->dev, tmp1, offset1);
>> +
>> +			return true;
>> +		}
>> +	}
>> +	return false;
>> +}
>> +
>>  static int m48t86_rtc_probe(struct platform_device *pdev)
>>  {
>>  	struct m48t86_rtc_info *info;
>> @@ -231,6 +262,11 @@ static int m48t86_rtc_probe(struct platform_device *pdev)
>>
>>  	dev_set_drvdata(&pdev->dev, info);
>>
>> +	if (!m48t86_verify_chip(pdev)) {
>> +		dev_info(&pdev->dev, "RTC not present\n");
>> +		return -ENODEV;
>> +	}
>> +
>>  	info->rtc = devm_rtc_device_register(&pdev->dev, "m48t86",
>>  					     &m48t86_rtc_ops, THIS_MODULE);
>>  	if (IS_ERR(info->rtc))
>> --
>> 2.10.0
>>
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Alexandre Belloni Feb. 21, 2017, 8:24 p.m. UTC | #5
On 21/02/2017 at 17:47:48 +0000, Alexander Clouter wrote:
> Thanks, not tested though, as I am lacking in time at the moment.  I
> probably am the only ts7800 user in the world using a mainline kernel
> too...so will only be punishing myself :-)
> 
> Acked-By: Alexander Clouter <alex+kernel@digriz.org.uk>
> 

Ok, then I've applied the whole series. I'll send that to Linus by the
end of the week.
diff mbox

Patch

diff --git a/drivers/rtc/rtc-m48t86.c b/drivers/rtc/rtc-m48t86.c
index 4dc4af4..41ac5d6 100644
--- a/drivers/rtc/rtc-m48t86.c
+++ b/drivers/rtc/rtc-m48t86.c
@@ -201,6 +201,37 @@  static ssize_t m48t86_nvram_write(struct file *filp, struct kobject *kobj,
 static BIN_ATTR(nvram, 0644, m48t86_nvram_read, m48t86_nvram_write,
 		M48T86_NVRAM_LEN);
 
+/*
+ * The RTC is an optional feature at purchase time on some Technologic Systems
+ * boards. Verify that it actually exists by checking if the last two bytes
+ * of the NVRAM can be changed.
+ *
+ * This is based on the method used in their rtc7800.c example.
+ */
+static bool m48t86_verify_chip(struct platform_device *pdev)
+{
+	unsigned int offset0 = M48T86_NVRAM_LEN - 2;
+	unsigned int offset1 = M48T86_NVRAM_LEN - 1;
+	unsigned char tmp0, tmp1;
+
+	tmp0 = m48t86_readb(&pdev->dev, offset0);
+	tmp1 = m48t86_readb(&pdev->dev, offset1);
+
+	m48t86_writeb(&pdev->dev, 0x00, offset0);
+	m48t86_writeb(&pdev->dev, 0x55, offset1);
+	if (m48t86_readb(&pdev->dev, offset1) == 0x55) {
+		m48t86_writeb(&pdev->dev, 0xaa, offset1);
+		if (m48t86_readb(&pdev->dev, offset1) == 0xaa &&
+		    m48t86_readb(&pdev->dev, offset0) == 0x00) {
+			m48t86_writeb(&pdev->dev, tmp0, offset0);
+			m48t86_writeb(&pdev->dev, tmp1, offset1);
+
+			return true;
+		}
+	}
+	return false;
+}
+
 static int m48t86_rtc_probe(struct platform_device *pdev)
 {
 	struct m48t86_rtc_info *info;
@@ -231,6 +262,11 @@  static int m48t86_rtc_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, info);
 
+	if (!m48t86_verify_chip(pdev)) {
+		dev_info(&pdev->dev, "RTC not present\n");
+		return -ENODEV;
+	}
+
 	info->rtc = devm_rtc_device_register(&pdev->dev, "m48t86",
 					     &m48t86_rtc_ops, THIS_MODULE);
 	if (IS_ERR(info->rtc))