[PATCH] mtd: bcm47xxsflash: implement chip polling

[PATCH] mtd: bcm47xxsflash: implement chip polling

[Rafał Miłecki]

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/devices/bcm47xxsflash.c |   56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
index 2f9e629..75f3cf0 100644
--- a/drivers/mtd/devices/bcm47xxsflash.c
+++ b/drivers/mtd/devices/bcm47xxsflash.c
@@ -1,6 +1,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include <linux/mtd/mtd.h>
 #include <linux/platform_device.h>
 #include <linux/bcma/bcma.h>
@@ -12,6 +13,58 @@ MODULE_DESCRIPTION("Serial flash driver for BCMA bus");
 
 static const char * const probes[] = { "bcm47xxpart", NULL };
 
+/**************************************************
+ * Various helpers
+ **************************************************/
+
+static void bcm47xxsflash_cmd(struct bcm47xxsflash *b47s, u32 opcode)
+{
+	int i;
+
+	bcma_cc_write32(b47s->bcma_cc, BCMA_CC_FLASHCTL,
+			BCMA_CC_FLASHCTL_START | opcode);
+	for (i = 0; i < 1000; i++) {
+		if (!(bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHCTL) &
+		      BCMA_CC_FLASHCTL_BUSY))
+			return;
+		cpu_relax();
+	}
+	pr_err("Control command failed (timeout)!\n");
+}
+
+static int bcm47xxsflash_poll(struct bcm47xxsflash *b47s, int timeout)
+{
+	unsigned long deadline = jiffies + timeout;
+
+	do {
+		switch (b47s->type) {
+		case BCM47XXSFLASH_TYPE_ST:
+			bcm47xxsflash_cmd(b47s, OPCODE_ST_RDSR);
+			if (!(bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHDATA)
+			      & SR_ST_WIP))
+				return 0;
+			break;
+		case BCM47XXSFLASH_TYPE_ATMEL:
+			bcm47xxsflash_cmd(b47s, OPCODE_AT_STATUS);
+			if (bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHDATA)
+			    & SR_AT_READY)
+				return 0;
+			break;
+		}
+
+		cpu_relax();
+		udelay(1);
+	} while (!time_after_eq(jiffies, deadline));
+
+	pr_err("Timeout waiting for flash to be ready!\n");
+
+	return -EBUSY;
+}
+
+/**************************************************
+ * MTD ops
+ **************************************************/
+
 static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
 			      size_t *retlen, u_char *buf)
 {
@@ -84,6 +137,9 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev)
 		goto err_dev_reg;
 	}
 
+	if (bcm47xxsflash_poll(b47s, HZ / 10))
+		pr_warn("Serial flash busy\n");
+
 	return 0;
 
 err_dev_reg:
-- 
1.7.10.4

Re: [PATCH] mtd: bcm47xxsflash: implement chip polling

[Hauke Mehrtens]

On 03/15/2013 11:31 AM, Rafał Miłecki wrote:
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/devices/bcm47xxsflash.c |   56 +++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
> index 2f9e629..75f3cf0 100644
> --- a/drivers/mtd/devices/bcm47xxsflash.c
> +++ b/drivers/mtd/devices/bcm47xxsflash.c
> @@ -1,6 +1,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/delay.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/platform_device.h>
>  #include <linux/bcma/bcma.h>
> @@ -12,6 +13,58 @@ MODULE_DESCRIPTION("Serial flash driver for BCMA bus");
>  
>  static const char * const probes[] = { "bcm47xxpart", NULL };
>  
> +/**************************************************
> + * Various helpers
> + **************************************************/
> +
> +static void bcm47xxsflash_cmd(struct bcm47xxsflash *b47s, u32 opcode)
> +{
> +	int i;
> +
> +	bcma_cc_write32(b47s->bcma_cc, BCMA_CC_FLASHCTL,
> +			BCMA_CC_FLASHCTL_START | opcode);

Why do you access the bcma write function directly? When you are adding
support for serial flash on ssb you have to double this and probably all
the other functions. I would suggest you using some bus abstraction like
it you did it in b43.

> +	for (i = 0; i < 1000; i++) {
> +		if (!(bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHCTL) &
> +		      BCMA_CC_FLASHCTL_BUSY))
> +			return;
> +		cpu_relax();
> +	}
> +	pr_err("Control command failed (timeout)!\n");
> +}
> +
> +static int bcm47xxsflash_poll(struct bcm47xxsflash *b47s, int timeout)
> +{
> +	unsigned long deadline = jiffies + timeout;
> +
> +	do {
> +		switch (b47s->type) {
> +		case BCM47XXSFLASH_TYPE_ST:
> +			bcm47xxsflash_cmd(b47s, OPCODE_ST_RDSR);
> +			if (!(bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHDATA)
> +			      & SR_ST_WIP))
> +				return 0;
> +			break;
> +		case BCM47XXSFLASH_TYPE_ATMEL:
> +			bcm47xxsflash_cmd(b47s, OPCODE_AT_STATUS);
> +			if (bcma_cc_read32(b47s->bcma_cc, BCMA_CC_FLASHDATA)
> +			    & SR_AT_READY)
> +				return 0;
> +			break;
> +		}
> +
> +		cpu_relax();
> +		udelay(1);
> +	} while (!time_after_eq(jiffies, deadline));
> +
> +	pr_err("Timeout waiting for flash to be ready!\n");
> +
> +	return -EBUSY;
> +}
> +
> +/**************************************************
> + * MTD ops
> + **************************************************/
> +
>  static int bcm47xxsflash_read(struct mtd_info *mtd, loff_t from, size_t len,
>  			      size_t *retlen, u_char *buf)
>  {
> @@ -84,6 +137,9 @@ static int bcm47xxsflash_bcma_probe(struct platform_device *pdev)
>  		goto err_dev_reg;
>  	}
>  
> +	if (bcm47xxsflash_poll(b47s, HZ / 10))
> +		pr_warn("Serial flash busy\n");
> +
>  	return 0;
>  
>  err_dev_reg:
> 

Re: [PATCH] mtd: bcm47xxsflash: implement chip polling

[Rafał Miłecki]

2013/3/15 Hauke Mehrtens <hauke@hauke-m.de>:
> On 03/15/2013 11:31 AM, Rafał Miłecki wrote:
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>>  drivers/mtd/devices/bcm47xxsflash.c |   56 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/devices/bcm47xxsflash.c
>> index 2f9e629..75f3cf0 100644
>> --- a/drivers/mtd/devices/bcm47xxsflash.c
>> +++ b/drivers/mtd/devices/bcm47xxsflash.c
>> @@ -1,6 +1,7 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/slab.h>
>> +#include <linux/delay.h>
>>  #include <linux/mtd/mtd.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/bcma/bcma.h>
>> @@ -12,6 +13,58 @@ MODULE_DESCRIPTION("Serial flash driver for BCMA bus");
>>
>>  static const char * const probes[] = { "bcm47xxpart", NULL };
>>
>> +/**************************************************
>> + * Various helpers
>> + **************************************************/
>> +
>> +static void bcm47xxsflash_cmd(struct bcm47xxsflash *b47s, u32 opcode)
>> +{
>> +     int i;
>> +
>> +     bcma_cc_write32(b47s->bcma_cc, BCMA_CC_FLASHCTL,
>> +                     BCMA_CC_FLASHCTL_START | opcode);
>
> Why do you access the bcma write function directly? When you are adding
> support for serial flash on ssb you have to double this and probably all
> the other functions. I would suggest you using some bus abstraction like
> it you did it in b43.

To be honest, I had bus abstraction in my code. I removed it because
of Artem's comments in last patchset. He said he doesn't have a proof
I'll ever add SSB support and told me to remove code that prepared
bcm47xxsflash for abstracting bus type.

Of course when adding support for SSB, I'll have to rewrite the code
I've just proposed. I also don't like this idea, but made it that way
to get it mainline. If we want to make it "the wise" way, with
designing code for bus abstraction at this point, we have to discuss
that with Artem.

@Artem: what do you think about this? Personally I'd prefer to start
designing bcm47xxsflash with keeping bus abstraction in mind.

-- 
Rafał

Re: [PATCH] mtd: bcm47xxsflash: implement chip polling

[Artem Bityutskiy]

On Fri, 2013-03-15 at 17:43 +0100, Rafał Miłecki wrote:
> To be honest, I had bus abstraction in my code. I removed it because
> of Artem's comments in last patchset. He said he doesn't have a proof
> I'll ever add SSB support and told me to remove code that prepared
> bcm47xxsflash for abstracting bus type.

I only asked to introduce it when you actually need it. I only objected
to adding preparations without the code which uses the preparations. I
do not really know the bcm47 HW, but of course if you have a HW resource
which is shared between subsystems, you should not use it directly from
MTD, you should have some kind of framework / accessors for the
resource.
 
-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: bcm47xxsflash: implement chip polling

[Rafał Miłecki]

2013/3/19 Artem Bityutskiy <dedekind1@gmail.com>:
> On Fri, 2013-03-15 at 17:43 +0100, Rafał Miłecki wrote:
>> To be honest, I had bus abstraction in my code. I removed it because
>> of Artem's comments in last patchset. He said he doesn't have a proof
>> I'll ever add SSB support and told me to remove code that prepared
>> bcm47xxsflash for abstracting bus type.
>
> I only asked to introduce it when you actually need it. I only objected
> to adding preparations without the code which uses the preparations. I
> do not really know the bcm47 HW, but of course if you have a HW resource
> which is shared between subsystems, you should not use it directly from
> MTD, you should have some kind of framework / accessors for the
> resource.

Thanks. I'll send you a V2 I prefer over this one today :)

-- 
Rafał