[PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[Alexander Shiyan]

Private field "clkdiv" is not used outside "probe", so there are
no reason to keep it in driver.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/w1/masters/mxc_w1.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
index 15c7251..5de3ca9 100644
--- a/drivers/w1/masters/mxc_w1.c
+++ b/drivers/w1/masters/mxc_w1.c
@@ -46,7 +46,6 @@
 
 struct mxc_w1_device {
 	void __iomem *regs;
-	unsigned int clkdiv;
 	struct clk *clk;
 	struct w1_bus_master bus_master;
 };
@@ -107,6 +106,7 @@ static int mxc_w1_probe(struct platform_device *pdev)
 {
 	struct mxc_w1_device *mdev;
 	struct resource *res;
+	unsigned int clkdiv;
 	int err = 0;
 
 	mdev = devm_kzalloc(&pdev->dev, sizeof(struct mxc_w1_device),
@@ -118,7 +118,7 @@ static int mxc_w1_probe(struct platform_device *pdev)
 	if (IS_ERR(mdev->clk))
 		return PTR_ERR(mdev->clk);
 
-	mdev->clkdiv = (clk_get_rate(mdev->clk) / 1000000) - 1;
+	clkdiv = (clk_get_rate(mdev->clk) / 1000000) - 1;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mdev->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -126,7 +126,7 @@ static int mxc_w1_probe(struct platform_device *pdev)
 		return PTR_ERR(mdev->regs);
 
 	clk_prepare_enable(mdev->clk);
-	__raw_writeb(mdev->clkdiv, mdev->regs + MXC_W1_TIME_DIVIDER);
+	__raw_writeb(clkdiv, mdev->regs + MXC_W1_TIME_DIVIDER);
 
 	mdev->bus_master.data = mdev;
 	mdev->bus_master.reset_bus = mxc_w1_ds2_reset_bus;
-- 
1.8.3.2

[PATCH 2/3] w1: mxc_w1: Add warning for base frequency calculation

[Alexander Shiyan]

Base frequency should be as close as possible to 1 MHz. This patch
adds warnings when clock is unreliable, according to i.MX datasheet.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/w1/masters/mxc_w1.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
index 5de3ca9..953168c 100644
--- a/drivers/w1/masters/mxc_w1.c
+++ b/drivers/w1/masters/mxc_w1.c
@@ -105,6 +105,7 @@ static u8 mxc_w1_ds2_touch_bit(void *data, u8 bit)
 static int mxc_w1_probe(struct platform_device *pdev)
 {
 	struct mxc_w1_device *mdev;
+	unsigned long clkrate;
 	struct resource *res;
 	unsigned int clkdiv;
 	int err = 0;
@@ -118,7 +119,16 @@ static int mxc_w1_probe(struct platform_device *pdev)
 	if (IS_ERR(mdev->clk))
 		return PTR_ERR(mdev->clk);
 
-	clkdiv = (clk_get_rate(mdev->clk) / 1000000) - 1;
+	clkrate = clk_get_rate(mdev->clk);
+	if (clkrate < 10000000)
+		dev_warn(&pdev->dev,
+			 "Low clock frequency causes improper function\n");
+
+	clkdiv = DIV_ROUND_CLOSEST(clkrate, 1000000);
+	clkrate /= clkdiv;
+	if ((clkrate < 980000) || (clkrate > 1020000))
+		dev_warn(&pdev->dev,
+			 "Incorrect time base frequency %lu Hz\n", clkrate);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mdev->regs = devm_ioremap_resource(&pdev->dev, res);
@@ -126,7 +136,7 @@ static int mxc_w1_probe(struct platform_device *pdev)
 		return PTR_ERR(mdev->regs);
 
 	clk_prepare_enable(mdev->clk);
-	__raw_writeb(clkdiv, mdev->regs + MXC_W1_TIME_DIVIDER);
+	__raw_writeb(clkdiv - 1, mdev->regs + MXC_W1_TIME_DIVIDER);
 
 	mdev->bus_master.data = mdev;
 	mdev->bus_master.reset_bus = mxc_w1_ds2_reset_bus;
-- 
1.8.3.2

[PATCH 3/3] w1: mxc_w1: Check the clk_prepare_enable() return value

[Alexander Shiyan]

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/w1/masters/mxc_w1.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/w1/masters/mxc_w1.c b/drivers/w1/masters/mxc_w1.c
index 953168c..1e5d94c 100644
--- a/drivers/w1/masters/mxc_w1.c
+++ b/drivers/w1/masters/mxc_w1.c
@@ -108,7 +108,7 @@ static int mxc_w1_probe(struct platform_device *pdev)
 	unsigned long clkrate;
 	struct resource *res;
 	unsigned int clkdiv;
-	int err = 0;
+	int err;
 
 	mdev = devm_kzalloc(&pdev->dev, sizeof(struct mxc_w1_device),
 			    GFP_KERNEL);
@@ -135,20 +135,23 @@ static int mxc_w1_probe(struct platform_device *pdev)
 	if (IS_ERR(mdev->regs))
 		return PTR_ERR(mdev->regs);
 
-	clk_prepare_enable(mdev->clk);
+	err = clk_prepare_enable(mdev->clk);
+	if (err)
+		return err;
+
 	__raw_writeb(clkdiv - 1, mdev->regs + MXC_W1_TIME_DIVIDER);
 
 	mdev->bus_master.data = mdev;
 	mdev->bus_master.reset_bus = mxc_w1_ds2_reset_bus;
 	mdev->bus_master.touch_bit = mxc_w1_ds2_touch_bit;
 
-	err = w1_add_master_device(&mdev->bus_master);
+	platform_set_drvdata(pdev, mdev);
 
+	err = w1_add_master_device(&mdev->bus_master);
 	if (err)
-		return err;
+		clk_disable_unprepare(mdev->clk);
 
-	platform_set_drvdata(pdev, mdev);
-	return 0;
+	return err;
 }
 
 /*
-- 
1.8.3.2

Re: [PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

On 11/29/2013 12:39 PM, Alexander Shiyan wrote:
> Private field "clkdiv" is not used outside "probe", so there are
> no reason to keep it in driver.

Where's the corresponding clk_disable_unprepare()?

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[Alexander Shiyan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 444 bytes --]

> On 11/29/2013 12:39 PM, Alexander Shiyan wrote:
> > Private field "clkdiv" is not used outside "probe", so there are
> > no reason to keep it in driver.
> 
> Where's the corresponding clk_disable_unprepare()?

I do not see any relation "clk_disable_unprepare" with this patch.

---
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On 11/29/2013 12:56 PM, Alexander Shiyan wrote:
>> On 11/29/2013 12:39 PM, Alexander Shiyan wrote:
>>> Private field "clkdiv" is not used outside "probe", so there are
>>> no reason to keep it in driver.
>>
>> Where's the corresponding clk_disable_unprepare()?
> 
> I do not see any relation "clk_disable_unprepare" with this patch.

Yes right, sorry for the noise.

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 1/3] w1: mxc_w1: Remove unused field "clkdiv" from private structure

[Evgeniy Polyakov]

Hi

29.11.2013, 15:40, "Alexander Shiyan" <shc_work@mail.ru>:
> Private field "clkdiv" is not used outside "probe", so there are
> no reason to keep it in driver.

The whole serie looks good, thank you
Greg, please pull it into your tree

> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>

Acked-by: Evgeniy Polyakov <zbr@ioremap.net>