[PATCH v1 1/2] mtd: docg3: add device-tree support

[PATCH v1 1/2] mtd: docg3: add device-tree support

[Robert Jarzmik]

Add device-tree support. This is straightforward as docg3 only uses the
standard IOMEM resources.

Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 drivers/mtd/devices/docg3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 91a169c..fc05f81 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/errno.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
 #include <linux/slab.h>
@@ -2126,10 +2127,17 @@ static int __exit docg3_release(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id docg3_dt_ids[] = {
+	{ .compatible = "sandisk,docg3" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, docg3_dt_ids);
+
 static struct platform_driver g3_driver = {
 	.driver		= {
 		.name	= "docg3",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(docg3_dt_ids),
 	},
 	.suspend	= docg3_suspend,
 	.resume		= docg3_resume,
-- 
2.0.0.rc2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/2] mtd: docg3: add device-tree documentation

[Robert Jarzmik]

Add documentation for the sandisk docg3 chip.

Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 Documentation/devicetree/bindings/mtd/docg3.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/docg3.txt

diff --git a/Documentation/devicetree/bindings/mtd/docg3.txt b/Documentation/devicetree/bindings/mtd/docg3.txt
new file mode 100644
index 0000000..3fcc163
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/docg3.txt
@@ -0,0 +1,14 @@
+Sandisk docg3 device
+====================
+
+The Sandisk (former MSystems) docg3 is a nand device of 64M to 256MB.
+
+Required properties:
+ - compatible: Should be "sandisk,docg3"
+ - reg: register base and size
+
+Example:
+	docg3 {
+		compatible = "sandisk,docg3";
+		reg = <0x0 0x2000>;
+	};
-- 
2.0.0.rc2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] mtd: docg3: add device-tree documentation

[Mark Rutland]

On Thu, Sep 25, 2014 at 11:33:16PM +0100, Robert Jarzmik wrote:
> Add documentation for the sandisk docg3 chip.
> 
> Signed-off-by: Robert Jarzmik <robert.jarzmik-GANU6spQydw@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  Documentation/devicetree/bindings/mtd/docg3.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/docg3.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/docg3.txt b/Documentation/devicetree/bindings/mtd/docg3.txt
> new file mode 100644
> index 0000000..3fcc163
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/docg3.txt
> @@ -0,0 +1,14 @@
> +Sandisk docg3 device
> +====================
> +
> +The Sandisk (former MSystems) docg3 is a nand device of 64M to 256MB.

I think that should be: "(formerly M-Systems)".

I'd rather we used the full name (DiskOnChip G3), as "docg3" is a
Linux-specific abbreviation. So I think the compatible string should be
something like "sandisk,diskonchip-g3". Arguably we should have
M-Systems as the vendor.

Are we able to detect the particular variant by reading registers on the
device? Are there any differences that we can probe dynamically (even if
we don't care about those at the moment)?

> +
> +Required properties:
> + - compatible: Should be "sandisk,docg3"
> + - reg: register base and size
> +
> +Example:
> +	docg3 {
> +		compatible = "sandisk,docg3";
> +		reg = <0x0 0x2000>;

There should be a unit-address on the node to match the address in the
first reg entry.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] mtd: docg3: add device-tree documentation

[Robert Jarzmik]

Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:

>> +The Sandisk (former MSystems) docg3 is a nand device of 64M to 256MB.
>
> I think that should be: "(formerly M-Systems)".
Right.

> I'd rather we used the full name (DiskOnChip G3), as "docg3" is a
> Linux-specific abbreviation. So I think the compatible string should be
> something like "sandisk,diskonchip-g3". Arguably we should have
> M-Systems as the vendor.
"sandisk,diskonchip-g3" : full ack, for v2

For M-Systems, it's as you wish. Just so that you have the broad view, this is
my understanding of M-Systems / Sandisk :
 - M-Systems creates several diskonchip chips, especially docg3
 - M-Systems is bought and absorbed by Sandisk
 - Sandisk creates and ships other diskonchip, under sandisk brand

Now I'll put in the compat whatever you advice for, I have no opinion on
that. I'm telling you this because I have another patch to submit for a camera
sensor made by Aptina. Aptina was absorbed by Micron, and the sensor was
released under Aptina/Micron brand (ie. Aptina team in Micron corp. if I
understood correctly).

Therefore, I'll take your advice for both sandisk/msystems and aptina/micron :)

> Are we able to detect the particular variant by reading registers on the
> device? Are there any differences that we can probe dynamically (even if
> we don't care about those at the moment)?

Yes, what defines a docg3 is :
 - a device mapped at address 0
 - a read of the chip id gives DOC_CHIPID_G3

But there is a catch : the read is not a simple memory read, it's a write to a
register to set the "register to read", then a read in the iospace. Doing this
implies you know you are in the iospace of a docg3 ...
>
>> +
>> +Required properties:
>> + - compatible: Should be "sandisk,docg3"
>> + - reg: register base and size
>> +
>> +Example:
>> +	docg3 {
>> +		compatible = "sandisk,docg3";
>> +		reg = <0x0 0x2000>;
>
> There should be a unit-address on the node to match the address in the
> first reg entry.
You mean "#address-cells = <1>;", right ?

Cheers.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] mtd: docg3: add device-tree documentation

[Mark Rutland]

On Fri, Sep 26, 2014 at 06:19:49PM +0100, Robert Jarzmik wrote:
> Mark Rutland <mark.rutland@arm.com> writes:
> 
> >> +The Sandisk (former MSystems) docg3 is a nand device of 64M to 256MB.
> >
> > I think that should be: "(formerly M-Systems)".
> Right.
> 
> > I'd rather we used the full name (DiskOnChip G3), as "docg3" is a
> > Linux-specific abbreviation. So I think the compatible string should be
> > something like "sandisk,diskonchip-g3". Arguably we should have
> > M-Systems as the vendor.
> "sandisk,diskonchip-g3" : full ack, for v2

ok.

> For M-Systems, it's as you wish. Just so that you have the broad view, this is
> my understanding of M-Systems / Sandisk :
>  - M-Systems creates several diskonchip chips, especially docg3

Ok, so I'd label those devices with an M-Systems vendor prefix
("m-systems", I guess, if we don't already have one).

>  - M-Systems is bought and absorbed by Sandisk

For the devices sold during this time where nothing has changed other
than the label, I'd keep the M-Systems vendor.

>  - Sandisk creates and ships other diskonchip, under sandisk brand

These new devices I would label with a sandisk vendor-prefix.

> Now I'll put in the compat whatever you advice for, I have no opinion on
> that. I'm telling you this because I have another patch to submit for a camera
> sensor made by Aptina. Aptina was absorbed by Micron, and the sensor was
> released under Aptina/Micron brand (ie. Aptina team in Micron corp. if I
> understood correctly).
> 
> Therefore, I'll take your advice for both sandisk/msystems and aptina/micron :)

I'd label something as the original brand it shipped under, unless
there's a compelling reason not to. We can add notes in the binding
documentation to make the bindings easier to find where a device has
been labelled by different manufacturers.

> > Are we able to detect the particular variant by reading registers on the
> > device? Are there any differences that we can probe dynamically (even if
> > we don't care about those at the moment)?
> 
> Yes, what defines a docg3 is :
>  - a device mapped at address 0
>  - a read of the chip id gives DOC_CHIPID_G3
> 
> But there is a catch : the read is not a simple memory read, it's a write to a
> register to set the "register to read", then a read in the iospace. Doing this
> implies you know you are in the iospace of a docg3 ...

I was more concerned with the identifying information that we can
acquire from the device than the precise sequence of steps that have to
be performed to extract that information.

You mention that the size of the flash is variable (it could be 64MB,
256MB, etc), but this isn't described in the binding. Therefore I assume
there is some mechanism by which I can query this from the device?

Are there other parameters that vary across instances? Even those for
which we currently don't care? If so, can these be queried from the
device?

> >
> >> +
> >> +Required properties:
> >> + - compatible: Should be "sandisk,docg3"
> >> + - reg: register base and size
> >> +
> >> +Example:
> >> +	docg3 {
> >> +		compatible = "sandisk,docg3";
> >> +		reg = <0x0 0x2000>;
> >
> > There should be a unit-address on the node to match the address in the
> > first reg entry.
> You mean "#address-cells = <1>;", right ?

No, the unit-address is the bit after the '@' on the node name. I'm
asking for:

docg3: flash@0 {
	...
	reg = <0x0 0x2000>;
	...
};

Thanks,
Mark.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 2/2] mtd: docg3: add device-tree documentation

[Robert Jarzmik]

Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> writes:

> Ok, so I'd label those devices with an M-Systems vendor prefix
> ("m-systems", I guess, if we don't already have one).
OK. We don't have it, so I'll add that patch for review too (I have submitted
one for sandisk, should have been m-systems ...)
>
>>  - M-Systems is bought and absorbed by Sandisk
>
> For the devices sold during this time where nothing has changed other
> than the label, I'd keep the M-Systems vendor.
OK.
>
>>  - Sandisk creates and ships other diskonchip, under sandisk brand
>
> These new devices I would label with a sandisk vendor-prefix.
OK.
>
>> Now I'll put in the compat whatever you advice for, I have no opinion on
>> that. I'm telling you this because I have another patch to submit for a camera
>> sensor made by Aptina. Aptina was absorbed by Micron, and the sensor was
>> released under Aptina/Micron brand (ie. Aptina team in Micron corp. if I
>> understood correctly).
>> 
>> Therefore, I'll take your advice for both sandisk/msystems and aptina/micron :)
>
> I'd label something as the original brand it shipped under, unless
> there's a compelling reason not to. We can add notes in the binding
> documentation to make the bindings easier to find where a device has
> been labelled by different manufacturers.
OK.
>
>> > Are we able to detect the particular variant by reading registers on the
>> > device? Are there any differences that we can probe dynamically (even if
>> > we don't care about those at the moment)?
>> 
>> Yes, what defines a docg3 is :
>>  - a device mapped at address 0
>>  - a read of the chip id gives DOC_CHIPID_G3
>> 
>> But there is a catch : the read is not a simple memory read, it's a write to a
>> register to set the "register to read", then a read in the iospace. Doing this
>> implies you know you are in the iospace of a docg3 ...
>
> I was more concerned with the identifying information that we can
> acquire from the device than the precise sequence of steps that have to
> be performed to extract that information.
>
> You mention that the size of the flash is variable (it could be 64MB,
> 256MB, etc), but this isn't described in the binding. Therefore I assume
> there is some mechanism by which I can query this from the device?
Ah you know what, you made me understand something in here.
As there is no spec of this device, I had created the driver by observing its
behaviour under another OS on a single chip, a 512 Mbits (ie. 64MBi) one.

I had assumed that the identification string, "0x200" (ie. DOC_CHIPID_G3) was
the identifier of a G3 chip. But you opened my eyes in here : it's the size of
the docg3 !

So your answer is :
 - a docg3 cannot be identified (from another diskonchip)
 - the variant of the docg3, ie. its size, can be identified (even if that
 doesn't change anything from a device-tree perspective). It's just something
 the driver should take into account.
 - I'm not aware of any other kind of variations nor any way to query them

> Are there other parameters that vary across instances? Even those for
> which we currently don't care? If so, can these be queried from the
> device?
Ah, no spec, no answer I'm afraid.
There might be variations in the ECC algorith, block size, etc ... but still no
spec.

I'm pretty sure the device has an interrupt line, and a requestor line for a DMA
most probably. No clue but heavy presomptions.

> No, the unit-address is the bit after the '@' on the node name. I'm
> asking for:
>
> docg3: flash@0 {
> 	...
> 	reg = <0x0 0x2000>;
> 	...
> };
Ah ok, I'll put that in v2.

Cheers.

-- 
Robert
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html