Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Tue, Nov 16, 2010 at 10:45:37PM +0100, Wolfram Sang wrote: > > > Even worse, I seem to recall that I had once seen a manufacturer increasing > > the > > page-size from one charge to the next without changing the part-number, so I > > got this feeling "you can't map pagesize to manufacturer/type" which I still > > have. Sadly, this was long ago, so I can't proof it right now. Will try to > > dig > > up some datasheets when in the office tomorrow. > > Had a look, couldn't find anything :( And now? Well, it seems that there are enough people in "pagesize" camp anyway, I'd say just go ahead with the current approach. :-) It's an additional information, so won't do any harm anyway... Thanks, -- Anton Vorontsov email: cbouatmai...@gmail.com irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
> Even worse, I seem to recall that I had once seen a manufacturer increasing > the > page-size from one charge to the next without changing the part-number, so I > got this feeling "you can't map pagesize to manufacturer/type" which I still > have. Sadly, this was long ago, so I can't proof it right now. Will try to dig > up some datasheets when in the office tomorrow. Had a look, couldn't find anything :( And now? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Mon, Nov 15, 2010 at 12:17:51PM -1000, Mitch Bradley wrote: > In general I think it's better to report parameter values directly, > instead of inferring them from manufacturer and part numbers. That > way you at least have a fighting chance of avoiding a kernel upgrade > when a part changes. I tend to agree. Although a fallback to deducing from the manufacturer / part id if the pagesize property is missing would be sensible. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
> > >I think you'd better drop the pagesize property altogether, and > > >instead make the compatible string more specific (if needed at > > >all. are there any 'catalyst,24c32' chips with pagesize != 32?) > > > > Microchip makes a 24c32 part that looks pretty similar to the > > catalyst part, but Microchip's has a 64-byte page size compared to > > Catalyst's 32. > > Well, when using microchip part, the compatible string would be > "microchip,24c32", correct? Then we have all the information > already, no need for the pagesize. Hmm, there are myriads of I2C eeproms out there, this table would be enourmous. Even worse, I seem to recall that I had once seen a manufacturer increasing the page-size from one charge to the next without changing the part-number, so I got this feeling "you can't map pagesize to manufacturer/type" which I still have. Sadly, this was long ago, so I can't proof it right now. Will try to dig up some datasheets when in the office tomorrow. In general, I2C EEPROMs are really a mess, the basic access method is the same, but except that everything else is possible :) Thus, this approach. Thus, this approach. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
In general I think it's better to report parameter values directly, instead of inferring them from manufacturer and part numbers. That way you at least have a fighting chance of avoiding a kernel upgrade when a part changes. Of course, that only works when the device tree is exported from the boot firmware instead of having to carry the device tree inside the kernel. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Mon, Nov 15, 2010 at 2:24 PM, Anton Vorontsov wrote:
> On Mon, Nov 15, 2010 at 11:06:44AM -1000, Mitch Bradley wrote:
> [...]
>> >> eep...@52 {
>> >> compatible = "catalyst,24c32";
>> >> reg =<0x52>;
>> >>+ pagesize =<32>;
>> >
>> >I think you'd better drop the pagesize property altogether, and
>> >instead make the compatible string more specific (if needed at
>> >all. are there any 'catalyst,24c32' chips with pagesize != 32?)
>>
>> Microchip makes a 24c32 part that looks pretty similar to the
>> catalyst part, but Microchip's has a 64-byte page size compared to
>> Catalyst's 32.
>
> Well, when using microchip part, the compatible string would be
> "microchip,24c32", correct? Then we have all the information
> already, no need for the pagesize.
>
>> It would probably be feasible to have a generic I2C EEPROM driver
>> that could handle many different parts, parameterized by total size,
>> block size, and page size.
The current at24.c driver is already parameterized; but part of the
parameter data is statically linked into the board support code.
>
> I guess it can do this already via I2C ID table. The problem
> is that I2C core is only using part ID (no vendor ID matching).
This could potentially be changed for at24 devices since the i2c
subsystem already matches by name. It would mean adding the vendor
prefix to all instantiations of the device in the kernel, although it
would mess up the current heuristic used by of_modalias_node() (not a
show-stopper).
>
> So, the current driver may just implement quirks like this:
>
> if (of_device_is_compatible(np, "catalyst,24c32"))
> pagesize = 32;
>
> Or, if it's some generic pattern, something like
>
> if (of_device_is_compatible_vendor(np, "catalyst"))
> pagesize /= 2;
This would get ugly in a hurry. It would be better to make it data
driven by searching for a better match in an of_device_id table so
that the workarounds don't grow over time and eventually achieve
sentience.
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Mon, Nov 15, 2010 at 11:06:44AM -1000, Mitch Bradley wrote:
[...]
> >>eep...@52 {
> >>compatible = "catalyst,24c32";
> >>reg =<0x52>;
> >>+ pagesize =<32>;
> >
> >I think you'd better drop the pagesize property altogether, and
> >instead make the compatible string more specific (if needed at
> >all. are there any 'catalyst,24c32' chips with pagesize != 32?)
>
> Microchip makes a 24c32 part that looks pretty similar to the
> catalyst part, but Microchip's has a 64-byte page size compared to
> Catalyst's 32.
Well, when using microchip part, the compatible string would be
"microchip,24c32", correct? Then we have all the information
already, no need for the pagesize.
> It would probably be feasible to have a generic I2C EEPROM driver
> that could handle many different parts, parameterized by total size,
> block size, and page size.
I guess it can do this already via I2C ID table. The problem
is that I2C core is only using part ID (no vendor ID matching).
So, the current driver may just implement quirks like this:
if (of_device_is_compatible(np, "catalyst,24c32"))
pagesize = 32;
Or, if it's some generic pattern, something like
if (of_device_is_compatible_vendor(np, "catalyst"))
pagesize /= 2;
Thanks,
--
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On 11/15/2010 7:32 AM, Anton Vorontsov wrote:
On Mon, Nov 15, 2010 at 06:25:16PM +0100, Wolfram Sang wrote:
Signed-off-by: Wolfram Sang
---
arch/powerpc/boot/dts/pcm030.dts |1 +
arch/powerpc/boot/dts/pcm032.dts |3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 8a4ec30..e7c36bc 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -259,6 +259,7 @@
eep...@52 {
compatible = "catalyst,24c32";
reg =<0x52>;
+ pagesize =<32>;
I think you'd better drop the pagesize property altogether, and
instead make the compatible string more specific (if needed at
all. are there any 'catalyst,24c32' chips with pagesize != 32?)
Microchip makes a 24c32 part that looks pretty similar to the catalyst
part, but Microchip's has a 64-byte page size compared to Catalyst's 32.
It would probably be feasible to have a generic I2C EEPROM driver that
could handle many different parts, parameterized by total size, block
size, and page size.
Thanks,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
On Mon, Nov 15, 2010 at 06:25:16PM +0100, Wolfram Sang wrote:
> Signed-off-by: Wolfram Sang
> ---
> arch/powerpc/boot/dts/pcm030.dts |1 +
> arch/powerpc/boot/dts/pcm032.dts |3 ++-
> 2 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/pcm030.dts
> b/arch/powerpc/boot/dts/pcm030.dts
> index 8a4ec30..e7c36bc 100644
> --- a/arch/powerpc/boot/dts/pcm030.dts
> +++ b/arch/powerpc/boot/dts/pcm030.dts
> @@ -259,6 +259,7 @@
> eep...@52 {
> compatible = "catalyst,24c32";
> reg = <0x52>;
> + pagesize = <32>;
I think you'd better drop the pagesize property altogether, and
instead make the compatible string more specific (if needed at
all. are there any 'catalyst,24c32' chips with pagesize != 32?)
Thanks,
--
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] powerpc: pcm030/032: add pagesize to dts
Signed-off-by: Wolfram Sang
---
arch/powerpc/boot/dts/pcm030.dts |1 +
arch/powerpc/boot/dts/pcm032.dts |3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 8a4ec30..e7c36bc 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -259,6 +259,7 @@
eep...@52 {
compatible = "catalyst,24c32";
reg = <0x52>;
+ pagesize = <32>;
};
};
diff --git a/arch/powerpc/boot/dts/pcm032.dts b/arch/powerpc/boot/dts/pcm032.dts
index 85d857a..e175e2c 100644
--- a/arch/powerpc/boot/dts/pcm032.dts
+++ b/arch/powerpc/boot/dts/pcm032.dts
@@ -257,8 +257,9 @@
reg = <0x51>;
};
eep...@52 {
- compatible = "at24,24c32";
+ compatible = "catalyst,24c32";
reg = <0x52>;
+ pagesize = <32>;
};
};
--
1.7.2.3
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
