RFC: book3e: DCR MMIO for IBM Blue Gene/Q

RFC: book3e: DCR MMIO for IBM Blue Gene/Q

[Jimi Xenidis]

I want to use the stuff in arch/platform/sysdev/dcr.c and I have a =
couple of questions:
1) anyone have a good devtree binding for this?
I'm thinking:
	bgq {
		#address-cells =3D <2>;
		#size-cells =3D <2>;
		...

		dcr {
			reg =3D <0x3ff 0xe0000000 0x0 0x08000000>;
			compatible =3D "ibm,bgq-dcr";
			dcr-controller;
			dcr-access-method =3D "mmio";
			dcr-mmio-stride =3D <??>;
		};
	};

I'm not sure what stride I will use since I have yet to figure out the =
DCR numbering in play at the moment, which will be painful.
Also, some clarity to the following would be nice:
	/* Stride is not properly defined yet, default to 0x10 for Axon =
*/
	p =3D of_get_property(dp, "dcr-mmio-stride", NULL);
	stride =3D (p =3D=3D NULL) ? 0x10 : *p;

	/* XXX FIXME: Which property name is to use of the 2 following ? =
*/
	p =3D of_get_property(dp, "dcr-mmio-range", NULL);
	if (p =3D=3D NULL)
		p =3D of_get_property(dp, "dcr-mmio-space", NULL);
	if (p =3D=3D NULL)
		goto done;

I guess my "reg" property is not useful to the code?
Is there a preference over dcr-mmio-range/space or in the absence of =
these to we should use "reg"?

2) I need 64 bit reads and writes
My intention is to have dcr_{read,write}_generic_{mmio,native} take an =
unsigned long for value all the way down.
For the native case, this would make is similar to mtspr and mfspr.
For the MMIO case, the selected in/out size would be based on "#ifdef =
__powerpc64__"

Thoughts?

-jx

Re: RFC: book3e: DCR MMIO for IBM Blue Gene/Q

[Benjamin Herrenschmidt]

On Sun, 2012-11-04 at 10:32 -0600, Jimi Xenidis wrote:
> I want to use the stuff in arch/platform/sysdev/dcr.c and I have a couple of questions:
> 1) anyone have a good devtree binding for this?

Not really, I think the cell stuff was the last we looked at this.

> I'm thinking:
> 	bgq {
> 		#address-cells = <2>;
> 		#size-cells = <2>;
> 		...
> 
> 		dcr {
> 			reg = <0x3ff 0xe0000000 0x0 0x08000000>;
> 			compatible = "ibm,bgq-dcr";
> 			dcr-controller;
> 			dcr-access-method = "mmio";
> 			dcr-mmio-stride = <??>;
> 		};
> 	};
> 
> I'm not sure what stride I will use since I have yet to figure out the DCR numbering in play at the moment, which will be painful.
> Also, some clarity to the following would be nice:
> 	/* Stride is not properly defined yet, default to 0x10 for Axon */
> 	p = of_get_property(dp, "dcr-mmio-stride", NULL);
> 	stride = (p == NULL) ? 0x10 : *p;
>
> 	/* XXX FIXME: Which property name is to use of the 2 following ? */
> 	p = of_get_property(dp, "dcr-mmio-range", NULL);
> 	if (p == NULL)
> 		p = of_get_property(dp, "dcr-mmio-space", NULL);
> 	if (p == NULL)
> 		goto done;

Not sure what SLOF did back then. dcr-mmio-range sounds better to me.

> I guess my "reg" property is not useful to the code?

Right, unless the DCR "bridge" has some registers of its own. It's also
handy to have as a "standard" way of representing the memory used by the
DCR bridge, tho that does mean duplication... but then it allows you to
have a nice unit address.

> Is there a preference over dcr-mmio-range/space or in the absence of these to we should use "reg"?
> 
> 2) I need 64 bit reads and writes
> My intention is to have dcr_{read,write}_generic_{mmio,native} take an unsigned long for value all the way down.
> For the native case, this would make is similar to mtspr and mfspr.
> For the MMIO case, the selected in/out size would be based on "#ifdef __powerpc64__"

As long as it doesn't break Axon...

Cheers,
Ben.

> Thoughts?
> 
> -jx
> 

Re: RFC: book3e: DCR MMIO for IBM Blue Gene/Q

[Jimi Xenidis]

On Nov 4, 2012, at 5:51 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org=
> wrote:

> On Sun, 2012-11-04 at 10:32 -0600, Jimi Xenidis wrote:
>> I want to use the stuff in arch/platform/sysdev/dcr.c and I have a couple=
 of questions:
>> 1) anyone have a good devtree binding for this?
>=20
> Not really, I think the cell stuff was the last we looked at this.
>=20
>> I'm thinking:
>>    bgq {
>>        #address-cells =3D <2>;
>>        #size-cells =3D <2>;
>>        ...
>>=20
>>        dcr {
>>            reg =3D <0x3ff 0xe0000000 0x0 0x08000000>;
>>            compatible =3D "ibm,bgq-dcr";
>>            dcr-controller;
>>            dcr-access-method =3D "mmio";
>>            dcr-mmio-stride =3D <??>;
>>        };
>>    };
>>=20
>> I'm not sure what stride I will use since I have yet to figure out the DC=
R numbering in play at the moment, which will be painful.
>> Also, some clarity to the following would be nice:
>>    /* Stride is not properly defined yet, default to 0x10 for Axon */
>>    p =3D of_get_property(dp, "dcr-mmio-stride", NULL);
>>    stride =3D (p =3D=3D NULL) ? 0x10 : *p;
>>=20
>>    /* XXX FIXME: Which property name is to use of the 2 following ? */
>>    p =3D of_get_property(dp, "dcr-mmio-range", NULL);
>>    if (p =3D=3D NULL)
>>        p =3D of_get_property(dp, "dcr-mmio-space", NULL);
>>    if (p =3D=3D NULL)
>>        goto done;
>=20
> Not sure what SLOF did back then. dcr-mmio-range sounds better to me.
>=20
>> I guess my "reg" property is not useful to the code?
>=20
> Right, unless the DCR "bridge" has some registers of its own. It's also
> handy to have as a "standard" way of representing the memory used by the
> DCR bridge, tho that does mean duplication... but then it allows you to
> have a nice unit address.

umm, was there a conclusion here? :-)
-jx

>=20
>> Is there a preference over dcr-mmio-range/space or in the absence of thes=
e to we should use "reg"?
>>=20
>> 2) I need 64 bit reads and writes
>> My intention is to have dcr_{read,write}_generic_{mmio,native} take an un=
signed long for value all the way down.
>> For the native case, this would make is similar to mtspr and mfspr.
>> For the MMIO case, the selected in/out size would be based on "#ifdef __p=
owerpc64__"
>=20
> As long as it doesn't break Axon...
>=20
> Cheers,
> Ben.
>=20
>> Thoughts?
>>=20
>> -jx
>=20
>=20

Re: RFC: book3e: DCR MMIO for IBM Blue Gene/Q

[Jimi Xenidis]

On Nov 4, 2012, at 9:23 PM, Jimi Xenidis wrote:

>=20
>=20
> On Nov 4, 2012, at 5:51 PM, Benjamin Herrenschmidt =
<benh@kernel.crashing.org> wrote:
>=20
>> On Sun, 2012-11-04 at 10:32 -0600, Jimi Xenidis wrote:
>>> I want to use the stuff in arch/platform/sysdev/dcr.c and I have a =
couple of questions:
>>> 1) anyone have a good devtree binding for this?
>>=20
>> Not really, I think the cell stuff was the last we looked at this.
>>=20
>>> I'm thinking:
>>>   bgq {
>>>       #address-cells =3D <2>;
>>>       #size-cells =3D <2>;
>>>       ...
>>>=20
>>>       dcr {
>>>           reg =3D <0x3ff 0xe0000000 0x0 0x08000000>;
>>>           compatible =3D "ibm,bgq-dcr";
>>>           dcr-controller;
>>>           dcr-access-method =3D "mmio";
>>>           dcr-mmio-stride =3D <??>;
>>>       };
>>>   };
>>>=20
>>> I'm not sure what stride I will use since I have yet to figure out =
the DCR numbering in play at the moment, which will be painful.
>>> Also, some clarity to the following would be nice:
>>>   /* Stride is not properly defined yet, default to 0x10 for Axon */
>>>   p =3D of_get_property(dp, "dcr-mmio-stride", NULL);
>>>   stride =3D (p =3D=3D NULL) ? 0x10 : *p;
>>>=20
>>>   /* XXX FIXME: Which property name is to use of the 2 following ? =
*/
>>>   p =3D of_get_property(dp, "dcr-mmio-range", NULL);
>>>   if (p =3D=3D NULL)
>>>       p =3D of_get_property(dp, "dcr-mmio-space", NULL);
>>>   if (p =3D=3D NULL)
>>>       goto done;
>>=20
>> Not sure what SLOF did back then. dcr-mmio-range sounds better to me.
>>=20
>>> I guess my "reg" property is not useful to the code?
>>=20
>> Right, unless the DCR "bridge" has some registers of its own. It's =
also
>> handy to have as a "standard" way of representing the memory used by =
the
>> DCR bridge, tho that does mean duplication... but then it allows you =
to
>> have a nice unit address.
>=20
> umm, was there a conclusion here? :-)
> -jx
>=20
>>=20
>>> Is there a preference over dcr-mmio-range/space or in the absence of =
these to we should use "reg"?
>>>=20
>>> 2) I need 64 bit reads and writes
>>> My intention is to have dcr_{read,write}_generic_{mmio,native} take =
an unsigned long for value all the way down.
>>> For the native case, this would make is similar to mtspr and mfspr.
>>> For the MMIO case, the selected in/out size would be based on =
"#ifdef __powerpc64__"
>>=20
>> As long as it doesn't break Axon...

So Axon, being 64-bit, would break with the changes proposed, since it =
is 64-bit bit and assumes 32bit accessors.
I'll simply add 64bit accessors for now.

-jx


>>=20
>> Cheers,
>> Ben.
>>=20
>>> Thoughts?
>>>=20
>>> -jx
>>=20
>>=20

Re: RFC: book3e: DCR MMIO for IBM Blue Gene/Q

[Benjamin Herrenschmidt]

On Mon, 2012-11-05 at 09:25 -0600, Jimi Xenidis wrote:
> So Axon, being 64-bit, would break with the changes proposed, since it
> is 64-bit bit and assumes 32bit accessors.
> I'll simply add 64bit accessors for now.

Well, either that, or you add a "64-bit" property to the DCR controller
node. IE. you keep the API "unsigned long" but whether it uses 32 or
64-bit accessors depends on that property.

Cheers,
Ben.