* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Grant Likely @ 2007-10-15 19:48 UTC (permalink / raw)
To: Scott Wood; +Cc: Jean Delvare, linuxppc-dev, Stefan Roese, i2c
In-Reply-To: <4713BE5E.3030406@freescale.com>
On 10/15/07, Scott Wood <scottwood@freescale.com> wrote:
> Grant Likely wrote:
> > On 10/15/07, Scott Wood <scottwood@freescale.com> wrote:
> >> On Mon, Oct 15, 2007 at 10:57:48AM -0600, Grant Likely wrote:
> >>> Segher is recommending that we use an aliases node as per the open
> >>> firmware example for this. I think in this case it would look
> >>> something like this (but I'm not the expert):
> >>>
> >>> aliases {
> >>> IIC0 = "/path/to/bus/iic@0x2000";
> >>> IIC1 = "/path/to/bus/iic@0x2000";
> >>> };
> >> I think this is overly complicated; something like linux,i2c-index in the
> >> i2c adapter node would be simpler.
> >
> > But not perhaps better. Enumeration is a system-wide thing. It does
> > make sense to group all the device enumerations in one place.
>
> For purposes of generating a Linux bus number, having to scan all
> aliases for a matching path and turn IIC0/IIC1 into a number is a bit silly.
>
> For associating a device node with a human readable label, I'd prefer a
> "label" property in the device node, rather than doing it backwards with
> aliases.
Here the corresponding problem; having to scan every device node to
make sure you don't assign a number already selected by another node
(in the case where one node is assigned a number and another is not).
At some point you're going to need to reverse map from number to
device. I'd rather scan a list of properties in aliases instead of
scanning the whole tree looking for all devices of the same type.
>
> > As per your point below; if all the i2c devices are children of the
> > adapter, then yes you are right that the bus number doesn't matter to
> > the user. But it *does* matter for things like serial and ethernet
> > ports.
>
> And a label property would be great for that. :-)
Not really; if the user needs to renumber devices; you don't want him
fiddling around in the hardware description. Just like the chosen
node; an aliases describes logical constructs, not physical ones. I
don't think this is any different from the linux,stdout-path property
in chosen.
>
> >> Though, I don't see what the problem with the original approach is, as long
> >> as the numbers are chosen in the same way when registering i2c clients based
> >> on the children of the adapter node. There's no concept in the hardware
> >> itself of a bus number.
> >
> > Even if you take this approach, the driver still need to know what bus
> > number to use (as per the i2c infrastructure). It is not sane for an
> > i2c bus driver to attempt to assign the bus number itself because it
> > could conflict with another arbitrarily chosen bus number from another
> > driver.
>
> Hence why global bus numbers suck. :-)
I'm not going to disagree with you there; but it's unavoidable.
> However, statically chosen numbers should only be done for platform
> devices, not dynamic things such as PCI, so in practice I don't think
> it'd be a problem.
So... in the I2C case, as long as all the i2c devices are in the
device tree and the i2c layer is responsible for actually handing out
the bus numbers; then yes I agree.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Scott Wood @ 2007-10-15 19:24 UTC (permalink / raw)
To: Grant Likely; +Cc: Jean Delvare, linuxppc-dev, Stefan Roese, i2c
In-Reply-To: <fa686aa40710151213r670bea49i63fa5f5402aa150d@mail.gmail.com>
Grant Likely wrote:
> On 10/15/07, Scott Wood <scottwood@freescale.com> wrote:
>> On Mon, Oct 15, 2007 at 10:57:48AM -0600, Grant Likely wrote:
>>> Segher is recommending that we use an aliases node as per the open
>>> firmware example for this. I think in this case it would look
>>> something like this (but I'm not the expert):
>>>
>>> aliases {
>>> IIC0 = "/path/to/bus/iic@0x2000";
>>> IIC1 = "/path/to/bus/iic@0x2000";
>>> };
>> I think this is overly complicated; something like linux,i2c-index in the
>> i2c adapter node would be simpler.
>
> But not perhaps better. Enumeration is a system-wide thing. It does
> make sense to group all the device enumerations in one place.
For purposes of generating a Linux bus number, having to scan all
aliases for a matching path and turn IIC0/IIC1 into a number is a bit silly.
For associating a device node with a human readable label, I'd prefer a
"label" property in the device node, rather than doing it backwards with
aliases.
> As per your point below; if all the i2c devices are children of the
> adapter, then yes you are right that the bus number doesn't matter to
> the user. But it *does* matter for things like serial and ethernet
> ports.
And a label property would be great for that. :-)
>> Though, I don't see what the problem with the original approach is, as long
>> as the numbers are chosen in the same way when registering i2c clients based
>> on the children of the adapter node. There's no concept in the hardware
>> itself of a bus number.
>
> Even if you take this approach, the driver still need to know what bus
> number to use (as per the i2c infrastructure). It is not sane for an
> i2c bus driver to attempt to assign the bus number itself because it
> could conflict with another arbitrarily chosen bus number from another
> driver.
Hence why global bus numbers suck. :-)
However, statically chosen numbers should only be done for platform
devices, not dynamic things such as PCI, so in practice I don't think
it'd be a problem.
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Scott Wood @ 2007-10-15 19:18 UTC (permalink / raw)
To: Eugene Surovegin; +Cc: Jean Delvare, Stefan Roese, i2c, linuxppc-dev
In-Reply-To: <20071015191118.GA9733@gate.ebshome.net>
Eugene Surovegin wrote:
> On Mon, Oct 15, 2007 at 01:53:40PM -0500, Scott Wood wrote:
>> Though, I don't see what the problem with the original approach is,
>> as long as the numbers are chosen in the same way when registering
>> i2c clients based on the children of the adapter node. There's no
>> concept in the hardware itself of a bus number.
>
> Huh? As far as I can tell, there is.
Where? Hardware != Documentation.
> Also, I want messages from the kernel mention something I can map to
> the real hw, e.g. fixed IIC device index, not some random number.
The OF node path should have a unit address that accomplishes that.
> I find it rather puzzling that instead people are trying to make this
> a non-issue as soon as it cannot be implemented easily with their new
> and shiny infrastructure.
"People" are not a monolithic entity. I never advocated using bus numbers.
-Scott
^ permalink raw reply
* Re: [PATCH v2] Device tree bindings for Xilinx devices
From: Arnd Bergmann @ 2007-10-15 19:21 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20071015155223.7403.39615.stgit@trillian.cg.shawcable.net>
On Monday 15 October 2007, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Here's my second version of xilinx device tree bindings. Please review
> and comment. I'd like to push these out to Paulus in the next couple
> of days.
There are a few more properties that I can imagine you might need.
Not sure if it makes sense specifying them now, but here are my thoughts:
> More devices will be defined as this spec matures.
>
> + l) Xilinx ML300 Framebuffer
> +
> + Simple framebuffer device from the ML300 reference design (also on the
> + ML403 reference design as well as others).
> +
> + Required properties:
> + - compatible : Must include "xilinx,ml300-fb"
> + - reg : offset and length of the framebuffer register set
> +
> + Optional properties:
> + - resolution : <xres yres> pixel resolution of framebuffer. Some
> + implementations use a different resolution. Default
> + is <d#640 d#480>
> + - virt-resolution : <xvirt yvirt> Size of framebuffer in memory.
> + Default is <d#1024 d#480>.
> + - rotate-display (empty) : rotate display 180 degrees.
rotate-display could be defined as something that allows 0/90/180/270
degrees, as well as mirroring, not just 180 degree rotation.
> + o) Xilinx Uartlite
> +
> + Xilinx uartlite devices are simple fixed speed serial ports. Uartlite
> + ports should be described in a node with the following properties.
> +
> + Requred properties:
> + - compatible : Must include "xilinx,uartlite"
> + - reg : offset and length of uartlite register set
> +
> + Recommended properties:
> + - interrupt-parent, interrupts : Connection of device irq signal.
> +
typically, serial ports include properties for current-speed and
clock-frequency. I guess it would be good to include at least one
of the two here.
Arnd <><
^ permalink raw reply
* Re: Linux booting problem on Xilinx ppc
From: Grant Likely @ 2007-10-15 19:20 UTC (permalink / raw)
To: aauer1; +Cc: linuxppc-embedded
In-Reply-To: <13219154.post@talk.nabble.com>
On 10/15/07, aauer1 <aauer1@gmx.at> wrote:
>
> Hello
>
> I'm also working with ML403 board from Xilinx and have the same problem as
> you. The boot process stops after decompressing with the message "Now
> booting the kernel". We also used gcc-4.1.0 for the cross compilation. So, I
> would be glad to know, which gcc version have you used to get a working
> kernel!
First step:
Get a JTAG debugger and look at the __log_buf address.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH v3 0/4] FEC - fast ethernet controller for mpc52xx
From: Grant Likely @ 2007-10-15 19:19 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linuxppc-dev, netdev, Domen Puncer
In-Reply-To: <4713BA1C.2050604@pobox.com>
On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> Domen Puncer wrote:
> > Hello!
> >
> > If there are no objections, I would like to get this merged
> > when bestcomm goes in (any time now?).
> >
> > It's split into four parts:
> > 1 - device tree
> > 2 - small bestcomm change
> > 3 - the actual driver
> > 4 - phy part of the driver
>
> patches #3 and #4 need to be combined together.
>
> Are the arch people OK with patches #1 and #2?
Yes, I'll be pushing both to Paulus shortly.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* RE: Linux booting problem on Xilinx ppc
From: Koss, Mike (Mission Systems) @ 2007-10-15 19:17 UTC (permalink / raw)
To: aauer1, linuxppc-embedded
In-Reply-To: <13219154.post@talk.nabble.com>
In house we use the gcc 4.1.2, glibc 2.3.6 combo with success.
-- Mike
-----Original Message-----
From: aauer1 [mailto:aauer1@gmx.at]=20
Sent: Monday, October 15, 2007 2:47 PM
To: linuxppc-embedded@ozlabs.org
Subject: Re: Linux booting problem on Xilinx ppc
Hello
I'm also working with ML403 board from Xilinx and have the same problem
as you. The boot process stops after decompressing with the message "Now
booting the kernel". We also used gcc-4.1.0 for the cross compilation.
So, I would be glad to know, which gcc version have you used to get a
working kernel!
Thanks,
Andreas
Junqiang Hu wrote:
>=20
>=20
> Hi Grant,
>=20
> Thank you so much for the reply! Fortunately I got it work now --=20
> it's the crosstool compiler problem. Originally I was using=20
> gcc-4.1.0, yet when compiling kernel 2.6.22, I noticed that it says=20
> gcc-4.1.0 will miscompile the kernel, so I changed to another version,
> and now I can let it go!
>=20
> Right now still not fully booted because of the SystemACE problem,=20
> or maybe the partition is not correct. I'm still working on it,=20
> hopefully to get it solved soon :-)
>=20
> Thanks,
> -J.
>=20
>=20
> Grant Likely-2 wrote:
>>=20
>> On 9/15/07, Junqiang Hu <jqhu936@yahoo.com> wrote:
>>>
>>>
>>> Dear friends,
>>>
>>> I'm trying to run Linux in AvNet (Memec)=20
>>> Xilinx-XC2VP50-EVKT-FF1152 board. The Linux version I'm using is=20
>>> 2.4; the cross-compiler is gcc-4.1.0, glibc 2.3.6. When booting the
kernel, it shows:
>>> loaded at: 00400000 004B51E4
>>> board data at: 00000000 00000018
>>> relocated to: 0040526C 00405284
>>> zimage at: 00405B2B 004B177C
>>> avail ram: 004B6000 60000000
>>=20
>> I strongly recommend moving to a 2.6 kernel. Recent mainline has=20
>> support for the Xilinx ppc built in.
>>>
>>> Linux/PPC load: console=3DttyS0,9600=20
>>> root=3D/dev/xsysace/disc0/part3 rw
>>> Uncompressing Linux...done.
>>> Now booting the kernel
>>>
>>> Then it hangs. First it seems to me that the "avail ram" is not=20
>>> correct, since I configured only 32MB SDRAM. Moreover, if it's=20
>>> first powered on, the end address of "avail ram" would be FFD9FBED.=20
>>> Then I tried to investigate the problem using xmd. When launched,=20
>>> it says:
>>=20
>> (If you're using u-boot) You might have a mismatch between the board=20
>> info structure used by u-boot and the one used by Linux.
>>=20
>> Also, you should use your debugger to inspect the __log_buf memory of
>> the kernel. A common problem is the kernel starts booting, but the=20
>> console is setup incorrectly and so you see nothing. But, you can=20
>> read the console output directly from memory if you look at the=20
>> __log_buf region (find the address in the System.map file; you might=20
>> need to subtract 0xC0000000 from the address to view the memory)
>>=20
>> Cheers,
>> g.
>>=20
>> --
>> Grant Likely, B.Sc., P.Eng.
>> Secret Lab Technologies Ltd.
>> grant.likely@secretlab.ca
>> (403) 399-0195
>> _______________________________________________
>> Linuxppc-embedded mailing list
>> Linuxppc-embedded@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>>=20
>>=20
>=20
>=20
--
View this message in context:
http://www.nabble.com/Linux-booting-problem-on-Xilinx-ppc-tf4449060.html
#a13219154
Sent from the linuxppc-embedded mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Grant Likely @ 2007-10-15 19:16 UTC (permalink / raw)
To: Eugene Surovegin; +Cc: Jean Delvare, Stefan Roese, i2c, linuxppc-dev
In-Reply-To: <20071015191118.GA9733@gate.ebshome.net>
On 10/15/07, Eugene Surovegin <ebs@ebshome.net> wrote:
> On Mon, Oct 15, 2007 at 01:53:40PM -0500, Scott Wood wrote:
> > Though, I don't see what the problem with the original approach is, as long
> > as the numbers are chosen in the same way when registering i2c clients based
> > on the children of the adapter node. There's no concept in the hardware
> > itself of a bus number.
>
> Huh? As far as I can tell, there is. Also, I want messages from the
> kernel mention something I can map to the real hw, e.g. fixed IIC
> device index, not some random number.
Yes, in the same way that there may be more than one on-chip serial or
ethernet controllers. However, it does not necessarily follow that
the *logical* bus number will match the way on chip devices are
numbered.
Example: Suppose you have a board with 2 chips which each include 2
i2c controllers. Each chip numbers them 1 & 2. So, which chip gets
1-2 and which one gets 3-4?
>
> This already works with the current OCP code, so if you want change
> it to a "superior" technology, please, make sure it provides the same
> functionality as trivial OCP one.
agreed
> I find it rather puzzling that instead people are trying to make
> this a non-issue as soon as it cannot be implemented easily with
> their new and shiny infrastructure.
No, it is a real problem; and not just for i2c. We need a solution for it.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Grant Likely @ 2007-10-15 19:13 UTC (permalink / raw)
To: Scott Wood; +Cc: Jean Delvare, linuxppc-dev, Stefan Roese, i2c
In-Reply-To: <20071015185340.GB4474@loki.buserror.net>
On 10/15/07, Scott Wood <scottwood@freescale.com> wrote:
> On Mon, Oct 15, 2007 at 10:57:48AM -0600, Grant Likely wrote:
> > Segher is recommending that we use an aliases node as per the open
> > firmware example for this. I think in this case it would look
> > something like this (but I'm not the expert):
> >
> > aliases {
> > IIC0 = "/path/to/bus/iic@0x2000";
> > IIC1 = "/path/to/bus/iic@0x2000";
> > };
>
> I think this is overly complicated; something like linux,i2c-index in the
> i2c adapter node would be simpler.
But not perhaps better. Enumeration is a system-wide thing. It does
make sense to group all the device enumerations in one place. It
eliminates two nodes trying to enumerate to the same device number and
since device numbers are something that the user will potentially want
to modify, it separates enumeration from hardware description.
As per your point below; if all the i2c devices are children of the
adapter, then yes you are right that the bus number doesn't matter to
the user. But it *does* matter for things like serial and ethernet
ports.
>
> Though, I don't see what the problem with the original approach is, as long
> as the numbers are chosen in the same way when registering i2c clients based
> on the children of the adapter node. There's no concept in the hardware
> itself of a bus number.
Even if you take this approach, the driver still need to know what bus
number to use (as per the i2c infrastructure). It is not sane for an
i2c bus driver to attempt to assign the bus number itself because it
could conflict with another arbitrarily chosen bus number from another
driver.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Eugene Surovegin @ 2007-10-15 19:11 UTC (permalink / raw)
To: Scott Wood; +Cc: Jean Delvare, Stefan Roese, i2c, linuxppc-dev
In-Reply-To: <20071015185340.GB4474@loki.buserror.net>
On Mon, Oct 15, 2007 at 01:53:40PM -0500, Scott Wood wrote:
> Though, I don't see what the problem with the original approach is, as long
> as the numbers are chosen in the same way when registering i2c clients based
> on the children of the adapter node. There's no concept in the hardware
> itself of a bus number.
Huh? As far as I can tell, there is. Also, I want messages from the
kernel mention something I can map to the real hw, e.g. fixed IIC
device index, not some random number.
This already works with the current OCP code, so if you want change
it to a "superior" technology, please, make sure it provides the same
functionality as trivial OCP one.
I find it rather puzzling that instead people are trying to make
this a non-issue as soon as it cannot be implemented easily with
their new and shiny infrastructure.
--
Eugene
^ permalink raw reply
* Re: [PATCH 1/2] [RFC][POWERPC] MPC8568E-MDS: create localbus node
From: Scott Wood @ 2007-10-15 19:10 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <3387D0D7-EBB8-4346-853D-75850159BCE4@kernel.crashing.org>
Kumar Gala wrote:
>
> On Oct 15, 2007, at 1:47 PM, Scott Wood wrote:
>
>> On Mon, Oct 15, 2007 at 07:57:30PM +0400, Anton Vorontsov wrote:
>>> On Mon, Oct 15, 2007 at 07:42:12PM +0400, Sergei Shtylyov wrote:
>>>> Is the entity described as "localbus" indeed so *board* specific?
>>>
>>> That's what booting-without-of.txt gives as an example.
>>
>> The board should have been left out of that. Your compatible should be
>> something like "fsl,mpc8568-localbus", possibly with some canonical
>> mpc85xx
>> chip as well, chosen to represent 85xx localbus in general.
>
> Lets be careful, there are at least two forms of localbus on 85xx.
Do you mean LAWBAR v. BRn/ORn (the manuals refer to the latter as
"localbus", so I'd be inclined to stick with that name, and call the
former mpc8568-lawbar or something), or some 85xx being different than
others?
-Scott
^ permalink raw reply
* Re: [PATCH 1/2] [RFC][POWERPC] MPC8568E-MDS: create localbus node
From: Kumar Gala @ 2007-10-15 19:08 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20071015184743.GA4474@loki.buserror.net>
On Oct 15, 2007, at 1:47 PM, Scott Wood wrote:
> On Mon, Oct 15, 2007 at 07:57:30PM +0400, Anton Vorontsov wrote:
>> On Mon, Oct 15, 2007 at 07:42:12PM +0400, Sergei Shtylyov wrote:
>>> Is the entity described as "localbus" indeed so *board* specific?
>>
>> That's what booting-without-of.txt gives as an example.
>
> The board should have been left out of that. Your compatible
> should be
> something like "fsl,mpc8568-localbus", possibly with some canonical
> mpc85xx
> chip as well, chosen to represent 85xx localbus in general.
Lets be careful, there are at least two forms of localbus on 85xx.
- k
^ permalink raw reply
* Re: [PATCH v3 0/4] FEC - fast ethernet controller for mpc52xx
From: Jeff Garzik @ 2007-10-15 19:06 UTC (permalink / raw)
To: Domen Puncer; +Cc: linuxppc-dev, netdev
In-Reply-To: <20071014075511.GC3000@nd47.coderock.org>
Domen Puncer wrote:
> Hello!
>
> If there are no objections, I would like to get this merged
> when bestcomm goes in (any time now?).
>
> It's split into four parts:
> 1 - device tree
> 2 - small bestcomm change
> 3 - the actual driver
> 4 - phy part of the driver
patches #3 and #4 need to be combined together.
Are the arch people OK with patches #1 and #2?
Jeff
^ permalink raw reply
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Jeff Garzik @ 2007-10-15 19:04 UTC (permalink / raw)
To: Josh Boyer; +Cc: netdev, linuxppc-dev
In-Reply-To: <20071015135959.5b603d7d@weaponx.rchland.ibm.com>
Josh Boyer wrote:
> On Mon, 15 Oct 2007 14:53:26 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
>>>> Seems sane to me -- ACK -- but we have multiple people sending me
>>>> patches for a single driver. That's normal for janitorial cleanups
>>>> across the whole tree, but discouraged when multiple people are actively
>>>> working on the same driver.
>>>>
>>>> Please coordinate, and have ONE person send me patches...
>>> Who else is sending you patches? Valentine is the only one I've seen
>>> send patches recently...
>> It's a zoo :)
>
> Wow, indeed.
>
>> Al Viro (3):
>> typo in ibm_newemac/rgmii.c
>
> Val sent this as well. Either one works.
>
>> skb->tail in ibm_newemac should be skb_tail_pointer()
>> ibm_newemac annotations (iomem, NULL noise)
>
> Ack on those.
>
>> David Gibson (1):
>> Device tree aware EMAC driver
>
> That's the initial commit :)
>
>> Michael Ellerman (3):
>> Update ibm_newemac to use dcr_host_t.base
>> Add dcr_host_t.base in dcr_read()/dcr_write()
>> Use dcr_host_t.base in dcr_unmap()
>
> Missed those, but I see you applied them which is good.
>
>> Roland Dreier (2):
>> ibm_new_emac: Nuke SET_MODULE_OWNER() use
>> ibm_emac: Convert to use napi_struct independent of struct net_device
>
> I never saw either of these. I'm also beginning to wonder if one of
> them broke things because I can't currently get ibm_newemac to work.
>
>> vbarshak@ru.mvista.com (1):
>> Fix typo in new EMAC driver.
>
> Same fix as Al's.
All those are what's upstream, except for the Michael Ellerman patches.
FWIW it was generated using
git log drivers/net/ibm_newemac | git shortlog
> Anyway, we can queue patches to this through me if you'd like.
I would ideally like a single active patch generator (even if they are
merely reviewed others work sometimes).
Outside of that, I'm hoping you and the other people listed making
changes will self-organize without my help :)
Jeff
^ permalink raw reply
* Re: [PATCH 2/2] i2c: Add devtree-aware iic support for PPC4xx
From: Scott Wood @ 2007-10-15 18:53 UTC (permalink / raw)
To: Grant Likely; +Cc: Jean Delvare, linuxppc-dev, Stefan Roese, i2c
In-Reply-To: <fa686aa40710150957s74693835g257b376adfdb4e90@mail.gmail.com>
On Mon, Oct 15, 2007 at 10:57:48AM -0600, Grant Likely wrote:
> Segher is recommending that we use an aliases node as per the open
> firmware example for this. I think in this case it would look
> something like this (but I'm not the expert):
>
> aliases {
> IIC0 = "/path/to/bus/iic@0x2000";
> IIC1 = "/path/to/bus/iic@0x2000";
> };
I think this is overly complicated; something like linux,i2c-index in the
i2c adapter node would be simpler.
Though, I don't see what the problem with the original approach is, as long
as the numbers are chosen in the same way when registering i2c clients based
on the children of the adapter node. There's no concept in the hardware
itself of a bus number.
-Scott
^ permalink raw reply
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Josh Boyer @ 2007-10-15 18:59 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linuxppc-dev
In-Reply-To: <4713B726.6080404@garzik.org>
On Mon, 15 Oct 2007 14:53:26 -0400
Jeff Garzik <jeff@garzik.org> wrote:
> >> Seems sane to me -- ACK -- but we have multiple people sending me
> >> patches for a single driver. That's normal for janitorial cleanups
> >> across the whole tree, but discouraged when multiple people are actively
> >> working on the same driver.
> >>
> >> Please coordinate, and have ONE person send me patches...
> >
> > Who else is sending you patches? Valentine is the only one I've seen
> > send patches recently...
>
> It's a zoo :)
Wow, indeed.
> Al Viro (3):
> typo in ibm_newemac/rgmii.c
Val sent this as well. Either one works.
> skb->tail in ibm_newemac should be skb_tail_pointer()
> ibm_newemac annotations (iomem, NULL noise)
Ack on those.
> David Gibson (1):
> Device tree aware EMAC driver
That's the initial commit :)
> Michael Ellerman (3):
> Update ibm_newemac to use dcr_host_t.base
> Add dcr_host_t.base in dcr_read()/dcr_write()
> Use dcr_host_t.base in dcr_unmap()
Missed those, but I see you applied them which is good.
> Roland Dreier (2):
> ibm_new_emac: Nuke SET_MODULE_OWNER() use
> ibm_emac: Convert to use napi_struct independent of struct net_device
I never saw either of these. I'm also beginning to wonder if one of
them broke things because I can't currently get ibm_newemac to work.
> vbarshak@ru.mvista.com (1):
> Fix typo in new EMAC driver.
Same fix as Al's.
Anyway, we can queue patches to this through me if you'd like.
josh
^ permalink raw reply
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Jeff Garzik @ 2007-10-15 18:53 UTC (permalink / raw)
To: Josh Boyer; +Cc: netdev, linuxppc-dev
In-Reply-To: <20071015134813.6aacdcca@weaponx.rchland.ibm.com>
Josh Boyer wrote:
> On Mon, 15 Oct 2007 14:27:23 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
>
>> Valentine Barshak wrote:
>>> This patch adds BCM5248 and Marvell 88E1111 PHY support to NEW EMAC driver.
>>> These PHY chips are used on PowerPC 440EPx boards.
>>> The PHY code is based on the previous work by Stefan Roese <sr@denx.de>
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
>>> ---
>>> drivers/net/ibm_newemac/phy.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 1 files changed, 39 insertions(+)
>>>
>>> --- linux.orig/drivers/net/ibm_newemac/phy.c 2007-06-15 21:45:18.000000000 +0400
>>> +++ linux/drivers/net/ibm_newemac/phy.c 2007-06-15 20:45:15.000000000 +0400
>>> @@ -306,8 +306,47 @@
>>> .ops = &cis8201_phy_ops
>>> };
>>>
>>> +static struct mii_phy_def bcm5248_phy_def = {
>>> +
>>> + .phy_id = 0x0143bc00,
>>> + .phy_id_mask = 0x0ffffff0,
>>> + .name = "BCM5248 10/100 SMII Ethernet",
>>> + .ops = &generic_phy_ops
>>> +};
>>> +
>>> +static int m88e1111_init(struct mii_phy *phy)
>>> +{
>>> + printk("%s: Marvell 88E1111 Ethernet\n", __FUNCTION__);
>>> + phy_write(phy, 0x14, 0x0ce3);
>>> + phy_write(phy, 0x18, 0x4101);
>>> + phy_write(phy, 0x09, 0x0e00);
>>> + phy_write(phy, 0x04, 0x01e1);
>>> + phy_write(phy, 0x00, 0x9140);
>>> + phy_write(phy, 0x00, 0x1140);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct mii_phy_ops m88e1111_phy_ops = {
>>> + .init = m88e1111_init,
>>> + .setup_aneg = genmii_setup_aneg,
>>> + .setup_forced = genmii_setup_forced,
>>> + .poll_link = genmii_poll_link,
>>> + .read_link = genmii_read_link
>>> +};
>>> +
>>> +static struct mii_phy_def m88e1111_phy_def = {
>>> +
>>> + .phy_id = 0x01410CC0,
>>> + .phy_id_mask = 0x0ffffff0,
>>> + .name = "Marvell 88E1111 Ethernet",
>>> + .ops = &m88e1111_phy_ops,
>>> +};
>>> +
>>> static struct mii_phy_def *mii_phy_table[] = {
>>> &cis8201_phy_def,
>>> + &bcm5248_phy_def,
>>> + &m88e1111_phy_def,
>>> &genmii_phy_def,
>> Seems sane to me -- ACK -- but we have multiple people sending me
>> patches for a single driver. That's normal for janitorial cleanups
>> across the whole tree, but discouraged when multiple people are actively
>> working on the same driver.
>>
>> Please coordinate, and have ONE person send me patches...
>
> Who else is sending you patches? Valentine is the only one I've seen
> send patches recently...
It's a zoo :)
Al Viro (3):
typo in ibm_newemac/rgmii.c
skb->tail in ibm_newemac should be skb_tail_pointer()
ibm_newemac annotations (iomem, NULL noise)
David Gibson (1):
Device tree aware EMAC driver
Michael Ellerman (3):
Update ibm_newemac to use dcr_host_t.base
Add dcr_host_t.base in dcr_read()/dcr_write()
Use dcr_host_t.base in dcr_unmap()
Roland Dreier (2):
ibm_new_emac: Nuke SET_MODULE_OWNER() use
ibm_emac: Convert to use napi_struct independent of struct net_device
vbarshak@ru.mvista.com (1):
Fix typo in new EMAC driver.
^ permalink raw reply
* Re: [PATCH v2] [POWERPC] ucc_geth: Fix build break introduced by commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0
From: Jeff Garzik @ 2007-10-15 18:51 UTC (permalink / raw)
To: Emil Medve; +Cc: netdev, akpm, leoli, linuxppc-dev
In-Reply-To: <1192455830-1028-1-git-send-email-Emilian.Medve@Freescale.com>
Emil Medve wrote:
> drivers/net/ucc_geth.c: In function 'ucc_geth_rx':
> drivers/net/ucc_geth.c:3483: error: 'dev' undeclared (first use in this function)
> drivers/net/ucc_geth.c:3483: error: (Each undeclared identifier is reported only once
> drivers/net/ucc_geth.c:3483: error: for each function it appears in.)
> make[2]: *** [drivers/net/ucc_geth.o] Error 1
>
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> ---
>
> Here is a convenient link for the culprit patch: http://git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.git;a=commit;h=09f75cd7bf13720738e6a196cc0107ce9a5bd5a0
>
> netdev-2.6> scripts/checkpatch.pl 0001-POWERPC-ucc_geth-Fix-build-break-introduced-by-co.patch
> Your patch has no obvious style problems and is ready for submission.
applied
^ permalink raw reply
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Josh Boyer @ 2007-10-15 18:48 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, linuxppc-dev
In-Reply-To: <4713B10B.5000607@garzik.org>
On Mon, 15 Oct 2007 14:27:23 -0400
Jeff Garzik <jeff@garzik.org> wrote:
> Valentine Barshak wrote:
> > This patch adds BCM5248 and Marvell 88E1111 PHY support to NEW EMAC driver.
> > These PHY chips are used on PowerPC 440EPx boards.
> > The PHY code is based on the previous work by Stefan Roese <sr@denx.de>
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
> > ---
> > drivers/net/ibm_newemac/phy.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 39 insertions(+)
> >
> > --- linux.orig/drivers/net/ibm_newemac/phy.c 2007-06-15 21:45:18.000000000 +0400
> > +++ linux/drivers/net/ibm_newemac/phy.c 2007-06-15 20:45:15.000000000 +0400
> > @@ -306,8 +306,47 @@
> > .ops = &cis8201_phy_ops
> > };
> >
> > +static struct mii_phy_def bcm5248_phy_def = {
> > +
> > + .phy_id = 0x0143bc00,
> > + .phy_id_mask = 0x0ffffff0,
> > + .name = "BCM5248 10/100 SMII Ethernet",
> > + .ops = &generic_phy_ops
> > +};
> > +
> > +static int m88e1111_init(struct mii_phy *phy)
> > +{
> > + printk("%s: Marvell 88E1111 Ethernet\n", __FUNCTION__);
> > + phy_write(phy, 0x14, 0x0ce3);
> > + phy_write(phy, 0x18, 0x4101);
> > + phy_write(phy, 0x09, 0x0e00);
> > + phy_write(phy, 0x04, 0x01e1);
> > + phy_write(phy, 0x00, 0x9140);
> > + phy_write(phy, 0x00, 0x1140);
> > +
> > + return 0;
> > +}
> > +
> > +static struct mii_phy_ops m88e1111_phy_ops = {
> > + .init = m88e1111_init,
> > + .setup_aneg = genmii_setup_aneg,
> > + .setup_forced = genmii_setup_forced,
> > + .poll_link = genmii_poll_link,
> > + .read_link = genmii_read_link
> > +};
> > +
> > +static struct mii_phy_def m88e1111_phy_def = {
> > +
> > + .phy_id = 0x01410CC0,
> > + .phy_id_mask = 0x0ffffff0,
> > + .name = "Marvell 88E1111 Ethernet",
> > + .ops = &m88e1111_phy_ops,
> > +};
> > +
> > static struct mii_phy_def *mii_phy_table[] = {
> > &cis8201_phy_def,
> > + &bcm5248_phy_def,
> > + &m88e1111_phy_def,
> > &genmii_phy_def,
>
> Seems sane to me -- ACK -- but we have multiple people sending me
> patches for a single driver. That's normal for janitorial cleanups
> across the whole tree, but discouraged when multiple people are actively
> working on the same driver.
>
> Please coordinate, and have ONE person send me patches...
Who else is sending you patches? Valentine is the only one I've seen
send patches recently...
josh
^ permalink raw reply
* Re: [PATCH 1/2] [RFC][POWERPC] MPC8568E-MDS: create localbus node
From: Scott Wood @ 2007-10-15 18:47 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
In-Reply-To: <20071015155730.GA30008@localhost.localdomain>
On Mon, Oct 15, 2007 at 07:57:30PM +0400, Anton Vorontsov wrote:
> On Mon, Oct 15, 2007 at 07:42:12PM +0400, Sergei Shtylyov wrote:
> > Is the entity described as "localbus" indeed so *board* specific?
>
> That's what booting-without-of.txt gives as an example.
The board should have been left out of that. Your compatible should be
something like "fsl,mpc8568-localbus", possibly with some canonical mpc85xx
chip as well, chosen to represent 85xx localbus in general.
-Scott
^ permalink raw reply
* Re: Linux booting problem on Xilinx ppc
From: aauer1 @ 2007-10-15 18:46 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <12696686.post@talk.nabble.com>
Hello
I'm also working with ML403 board from Xilinx and have the same problem as
you. The boot process stops after decompressing with the message "Now
booting the kernel". We also used gcc-4.1.0 for the cross compilation. So, I
would be glad to know, which gcc version have you used to get a working
kernel!
Thanks,
Andreas
Junqiang Hu wrote:
>
>
> Hi Grant,
>
> Thank you so much for the reply! Fortunately I got it work now -- it's
> the crosstool compiler problem. Originally I was using gcc-4.1.0, yet
> when compiling kernel 2.6.22, I noticed that it says gcc-4.1.0 will
> miscompile the kernel, so I changed to another version, and now I can let
> it go!
>
> Right now still not fully booted because of the SystemACE problem, or
> maybe the partition is not correct. I'm still working on it, hopefully to
> get it solved soon :-)
>
> Thanks,
> -J.
>
>
> Grant Likely-2 wrote:
>>
>> On 9/15/07, Junqiang Hu <jqhu936@yahoo.com> wrote:
>>>
>>>
>>> Dear friends,
>>>
>>> I'm trying to run Linux in AvNet (Memec) Xilinx-XC2VP50-EVKT-FF1152
>>> board. The Linux version I'm using is 2.4; the cross-compiler is
>>> gcc-4.1.0, glibc 2.3.6. When booting the kernel, it shows:
>>> loaded at: 00400000 004B51E4
>>> board data at: 00000000 00000018
>>> relocated to: 0040526C 00405284
>>> zimage at: 00405B2B 004B177C
>>> avail ram: 004B6000 60000000
>>
>> I strongly recommend moving to a 2.6 kernel. Recent mainline has
>> support for the Xilinx ppc built in.
>>>
>>> Linux/PPC load: console=ttyS0,9600 root=/dev/xsysace/disc0/part3
>>> rw
>>> Uncompressing Linux...done.
>>> Now booting the kernel
>>>
>>> Then it hangs. First it seems to me that the "avail ram" is not correct,
>>> since I configured only 32MB SDRAM. Moreover, if it's first powered on,
>>> the
>>> end address of "avail ram" would be FFD9FBED. Then I tried to
>>> investigate
>>> the problem using xmd. When launched, it says:
>>
>> (If you're using u-boot) You might have a mismatch between the board
>> info structure used by u-boot and the one used by Linux.
>>
>> Also, you should use your debugger to inspect the __log_buf memory of
>> the kernel. A common problem is the kernel starts booting, but the
>> console is setup incorrectly and so you see nothing. But, you can
>> read the console output directly from memory if you look at the
>> __log_buf region (find the address in the System.map file; you might
>> need to subtract 0xC0000000 from the address to view the memory)
>>
>> Cheers,
>> g.
>>
>> --
>> Grant Likely, B.Sc., P.Eng.
>> Secret Lab Technologies Ltd.
>> grant.likely@secretlab.ca
>> (403) 399-0195
>> _______________________________________________
>> Linuxppc-embedded mailing list
>> Linuxppc-embedded@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-embedded
>>
>>
>
>
--
View this message in context: http://www.nabble.com/Linux-booting-problem-on-Xilinx-ppc-tf4449060.html#a13219154
Sent from the linuxppc-embedded mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH take2] [POWERPC] i2c: adds support for i2c bus on 8xx
From: Scott Wood @ 2007-10-15 18:31 UTC (permalink / raw)
To: Jochen Friedrich
Cc: Carsten Juttner, linux-kernel, i2c, linuxppc-embedded@ozlabs.org
In-Reply-To: <47134A94.60606@scram.de>
On Mon, Oct 15, 2007 at 01:10:12PM +0200, Jochen Friedrich wrote:
> Using the port of 2.4 code from Vitaly Bordug <vitb@kernel.crashing.org>
> and the actual algorithm used by the i2c driver of the DBox code on
> cvs.tuxboc.org from Tmbinc, Gillem (htoa@gmx.net). Renamed
> i2c-algo-8xx.c to i2c-algo-cpm.c and i2c-rpx.c to i2c-8xx.c. Added
> original i2c-rpx.c as i2c-8xx-ppc.c for pre-OF (arch ppc) devices.
Do we really need to be adding features for arch/ppc at this point? It'll
be going away in June. arch/ppc-specific things outside of arch/ppc itself
will also be more likely to be missed in the removal.
Also, please post inline rather than as an attachment; attachments are
harder to quote in a reply.
> diff --git a/arch/powerpc/boot/dts/mpc885ads.dts b/arch/powerpc/boot/dts/mpc885ads.dts
> index 8848e63..a526c02 100644
> --- a/arch/powerpc/boot/dts/mpc885ads.dts
> +++ b/arch/powerpc/boot/dts/mpc885ads.dts
> @@ -213,6 +213,15 @@
> fsl,cpm-command = <0080>;
> linux,network-index = <2>;
> };
> +
> + i2c@860 {
> + device_type = "i2c";
No device_type.
> + compatible = "fsl-i2c-cpm";
Should be fsl,cpm-i2c. Is cpm2 i2c the same? If not, it should be
fsl,cpm1-i2c. It's probably best to specify it anyway, along with
fsl,mpc885-i2c.
> diff --git a/arch/powerpc/platforms/8xx/mpc885ads_setup.c b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> index 2cf1b6a..350018b 100644
> --- a/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> +++ b/arch/powerpc/platforms/8xx/mpc885ads_setup.c
> @@ -175,6 +175,12 @@ static void __init init_ioports(void)
>
> /* Set FEC1 and FEC2 to MII mode */
> clrbits32(&mpc8xx_immr->im_cpm.cp_cptr, 0x00000180);
> +
> +#ifdef CONFIG_I2C_8XX
> + setbits32(&mpc8xx_immr->im_cpm.cp_pbpar, 0x00000030);
> + setbits32(&mpc8xx_immr->im_cpm.cp_pbdir, 0x00000030);
> + setbits16(&mpc8xx_immr->im_cpm.cp_pbodr, 0x0030);
> +#endif
Please add this to mpc885ads_pins, rather than poking the registers
directly. The relevant lines are:
{CPM_PORTB, 26, CPM_PIN_OUTPUT},
{CPM_PORTB, 27, CPM_PIN_OUTPUT},
> +static const char *i2c_regs = "regs";
> +static const char *i2c_pram = "pram";
> +static const char *i2c_irq = "interrupt";
> +
> +static int __init fsl_i2c_cpm_of_init(void)
> +{
> + struct device_node *np;
> + unsigned int i;
> + struct platform_device *i2c_dev;
> + int ret;
> +
> + for (np = NULL, i = 0;
> + (np = of_find_compatible_node(np, "i2c", "fsl-i2c-cpm")) != NULL;
> + i++) {
Why not just make an of_platform device instead of this glue code?
> diff --git a/drivers/i2c/algos/Kconfig b/drivers/i2c/algos/Kconfig
> index 014dfa5..7a8200e 100644
> --- a/drivers/i2c/algos/Kconfig
> +++ b/drivers/i2c/algos/Kconfig
> @@ -14,6 +14,18 @@ config I2C_ALGOBIT
> This support is also available as a module. If so, the module
> will be called i2c-algo-bit.
>
> +config I2C_ALGOCPM
> + tristate "I2C MPC8xx CPM and MPC8260 CPM2 interfaces"
> + depends on ( 8xx || 8260 ) && I2C
> + help
> + CPM I2C Algorithm, supports the CPM I2C interface for mpc8xx
> + and mpc8260 CPUs. Say Y if you own a CPU of this class
What if I'm just borrowing it? :-)
> +static irqreturn_t cpm_iic_interrupt(int irq, void *dev_id)
> +{
> + struct i2c_adapter *adap;
> + struct i2c_algo_cpm_data *cpm;
> + i2c8xx_t *i2c;
> + int i;
> +
> + adap = (struct i2c_adapter *) dev_id;
> + cpm = adap->algo_data;
> + i2c = cpm->i2c;
> +
> + /* Clear interrupt.
> + */
> + i = in_8(&i2c->i2c_i2cer);
> + out_8(&i2c->i2c_i2cer, i);
> +
> + if (cpm_debug)
> + dev_dbg(&adap->dev, "Interrupt: %x\n", i);
> +
> + /* Get 'me going again.
> + */
> + wake_up_interruptible(&cpm->iic_wait);
> +
> + return IRQ_HANDLED;
> +}
Should only return IRQ_HANDLED if the event register was non-zero.
> +static int cpm_iic_init(struct i2c_adapter *adap)
> +{
> + struct i2c_algo_cpm_data *cpm = adap->algo_data;
> + iic_t *iip = cpm->iip;
> + i2c8xx_t *i2c = cpm->i2c;
> + unsigned char brg;
> + int ret, i;
> +
> + if (cpm_debug)
> + dev_dbg(&adap->dev, "cpm_iic_init()\n");
Can you fold the if statement into a macro? Or just do a raw dev_dbg with
no test, like most drivers do.
> + ret = 0;
> + init_waitqueue_head(&cpm->iic_wait);
> + mutex_init(&cpm->iic_mutex);
> + /* Initialize the parameter ram.
> + * We need to make sure many things are initialized to zero,
> + * especially in the case of a microcode patch.
> + */
> + out_be32(&iip->iic_rstate, 0);
> + out_be32(&iip->iic_rdp, 0);
> + out_be16(&iip->iic_rbptr, 0);
> + out_be16(&iip->iic_rbc, 0);
> + out_be32(&iip->iic_rxtmp, 0);
> + out_be32(&iip->iic_tstate, 0);
> + out_be32(&iip->iic_tdp, 0);
> + out_be16(&iip->iic_tbptr, 0);
> + out_be16(&iip->iic_tbc, 0);
> + out_be32(&iip->iic_txtmp, 0);
This appears to be done twice, here and in cpm_reset_iic_params.
> + u16 v = mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_INIT_TRX) | CPM_CR_FLG;
Should use fsl,cpm-command rather than hardcoded constants.
> + /* Select an arbitrary address. Just make sure it is unique.
> + */
> + out_8(&i2c->i2c_i2add, 0xfe);
It's a 7-bit address... and are you sure that 0x7e is unique? Does this
driver even support slave operation?
> + /* Make clock run at 60 kHz.
> + */
> + /* brg = ppc_proc_freq / (32 * 2 * 60000) - 3; */
> + brg = 7;
> + out_8(&i2c->i2c_i2brg, brg);
Why is this hardcoded?
> + out_8(&i2c->i2c_i2mod, 0x00);
> + out_8(&i2c->i2c_i2com, 0x01); /* Master mode */
> +
> + /* Disable interrupts.
> + */
> + out_8(&i2c->i2c_i2cmr, 0);
> + out_8(&i2c->i2c_i2cer, 0xff);
> +
> + /* Allocate TX and RX buffers */
> + for (i = 0; i < CPM_MAXBD; i++) {
> + cpm->rxbuf[i] = (unsigned char *)dma_alloc_coherent(
> + NULL, CPM_MAX_READ + 1, &cpm->rxdma[i], GFP_KERNEL);
> + if (!cpm->rxbuf[i]) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + cpm->txbuf[i] = (unsigned char *)dma_alloc_coherent(
> + NULL, CPM_MAX_READ + 1, &cpm->txdma[i], GFP_KERNEL);
> + if (!cpm->txbuf[i]) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + }
> +
> + /* Install interrupt handler.
> + */
> + if (request_irq(cpm->irq, cpm_iic_interrupt, 0, "8xx_i2c", adap)) {
cpm-i2c, not 8xx.
> + ret = -EIO;
> + goto out;
> + }
> +
> + return 0;
> +out:
> + for (i = 0; i < CPM_MAXBD; i++) {
> + if (cpm->rxbuf[i]) dma_free_coherent(NULL, CPM_MAX_READ + 1,
> + cpm->rxbuf[i], cpm->rxdma[i]);
> + if (cpm->txbuf[i]) dma_free_coherent(NULL, CPM_MAX_READ + 1,
> + cpm->txbuf[i], cpm->txdma[i]);
> + }
Please put a newline between the if test and the if body.
> +static void force_close(struct i2c_adapter *adap)
> +{
> + struct i2c_algo_cpm_data *cpm = adap->algo_data;
> + i2c8xx_t *i2c = cpm->i2c;
> + if (cpm->reloc == 0) { /* micro code disabled */
> + cpm8xx_t *cp = cpm->cp;
> + u16 v =
> + mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_CLOSE_RXBD) | CPM_CR_FLG;
Why only if there's no microcode relocation?
> +static void cpm_parse_message(struct i2c_adapter *adap, struct i2c_msg *pmsg,
> + int num, int tx, int rx)
> +{
> + cbd_t *tbdf, *rbdf;
> + u_char addr;
> + u_char *tb;
> + u_char *rb;
> + struct i2c_algo_cpm_data *cpm = adap->algo_data;
> + iic_t *iip = cpm->iip;
> + int i, dscan;
> +
> + tbdf = (cbd_t *) cpm_dpram_addr(in_be16(&iip->iic_tbase));
> + rbdf = (cbd_t *) cpm_dpram_addr(in_be16(&iip->iic_rbase));
> +
> + /* This chip can't do zero lenth writes. However, the i2c core uses
s/lenth/length/
> + if (pmsg->flags & I2C_M_RD) {
> + if (cpm_debug)
> + dev_dbg(&adap->dev, "rx sc %04x, rx sc %04x\n",
> + tbdf[tx].cbd_sc, rbdf[rx].cbd_sc);
> + if (tbdf[tx].cbd_sc & BD_SC_NAK) {
> + if (cpm_debug)
> + dev_dbg(&adap->dev, "IIC read; no ack\n");
> + if (pmsg->flags & I2C_M_IGNORE_NAK)
> + return 0;
> + else
> + return -EREMOTEIO;
s/EREMOTEIO/EIO/
> +static int cpm_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + struct i2c_algo_cpm_data *cpm = adap->algo_data;
> + i2c8xx_t *i2c = cpm->i2c;
> + iic_t *iip = cpm->iip;
> + struct i2c_msg *pmsg, *rmsg;
> + int ret, i;
> + int tptr;
> + int rptr;
> + cbd_t *tbdf, *rbdf;
> +
> + if (num > CPM_MAXBD)
> + return -EREMOTEIO;
> +
> + /* Check if we have any oversized READ requests */
> + for (i = 0; i < num; i++) {
> + pmsg = &msgs[i];
> + if (pmsg->len >= CPM_MAX_READ)
> + return -EREMOTEIO;
> + }
s/EREMOTEIO/EINVAL/
> +
> + mutex_lock(&cpm->iic_mutex);
> +
> + /* check for and use a microcode relocation patch */
> + if (cpm->reloc)
> + cpm_reset_iic_params(cpm);
On every transfer?
> + while (tptr < num) {
> + /* Check for outstanding messages */
> + dev_dbg(&adap->dev, "test ready.\n");
> + if (!(tbdf[tptr].cbd_sc & BD_SC_READY)) {
> + dev_dbg(&adap->dev, "ready.\n");
> + rmsg = &msgs[tptr];
> + ret = cpm_check_message(adap, rmsg, tptr, rptr);
> + tptr++;
> + if (rmsg->flags & I2C_M_RD)
> + rptr++;
> + if (ret) {
> + force_close(adap);
> + mutex_unlock(&cpm->iic_mutex);
> + return -EREMOTEIO;
s/-EREMOTEIO/ret/
> +config I2C_8XX
> + tristate "MPC8xx CPM with Open Firmware"
It's not really Open Firmware... just a flat device tree.
> diff --git a/drivers/i2c/busses/i2c-8xx.c b/drivers/i2c/busses/i2c-8xx.c
> new file mode 100644
> index 0000000..c38b52e
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-8xx.c
> @@ -0,0 +1,170 @@
> +/*
> + * Embedded Planet RPX Lite MPC8xx CPM I2C interface.
> + * Copyright (c) 1999 Dan Malek (dmalek@jlc.net).
> + *
> + * moved into proper i2c interface;
> + * Brad Parker (brad@heeltoe.com)
> + *
> + * (C) 2007 Montavista Software, Inc.
> + * Vitaly Bordug <vitb@kernel.crashing.org>
> + *
> + * RPX lite specific parts of the i2c interface
> + * Update: There actually isn't anything RPXLite-specific about this module.
> + * This should work for most any 8xx board. The console messages have been
> + * changed to eliminate RPXLite references.
So let's change the title at the top of the file...
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/stddef.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-algo-cpm.h>
> +#include <linux/of_device.h>
Why are you including platform_device, and running glue code in fsl_soc.c,
if this is an of_platform driver?
> +#include <linux/of_platform.h>
> +#include <asm/mpc8xx.h>
> +#include <asm/commproc.h>
> +#include <asm/fs_pd.h>
> +
> +
> +struct m8xx_i2c {
> + char *base;
> + struct of_device *ofdev;
> + struct i2c_adapter adap;
> + struct i2c_algo_cpm_data *algo_8xx;
> +};
> +
> +static struct i2c_algo_cpm_data m8xx_data;
> +
> +static const struct i2c_adapter m8xx_ops = {
> + .owner = THIS_MODULE,
> + .name = "i2c-8xx",
> + .id = I2C_HW_MPC8XX_EPON,
> + .algo_data = &m8xx_data,
> + .dev.parent = &platform_bus,
> + .class = I2C_CLASS_HWMON,
> +};
It's not on the platform bus; it's on the soc of_platform bus. Why is this
embedded in the i2c_adapter struct anyway? i2c_adapter should hold a
pointer to whatever the probe function gives you.
> + data->irq = irq_of_parse_and_map(np, 0);
> + if (data->irq < 0)
> + return -EINVAL;
> +
> + if (of_address_to_resource(np, 1, &r))
> + return -EINVAL;
> +
> + data->iip = ioremap(r.start, r.end - r.start + 1);
> + if (data->iip == NULL)
> + return -EINVAL;
> +
> + /* Check for and use a microcode relocation patch.
> + */
> + data->reloc = data->iip->iic_rpbase;
> + if (data->reloc)
> + data->iip = (iic_t *)&cp->cp_dpmem[data->iip->iic_rpbase];
> +
> + if (of_address_to_resource(np, 0, &r))
> + return -EINVAL;
> +
> + data->i2c = ioremap(r.start, r.end - r.start + 1);
Use of_iomap.
> + if (data->i2c == NULL)
> + return -EINVAL;
> +
> + /* Allocate space for two transmit and two receive buffer
> + * descriptors in the DP ram.
> + */
> + data->dp_addr = cpm_dpalloc(sizeof(cbd_t) * 4, 8);
Please use the new muram_dpalloc name.
> + if (!data->dp_addr)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int i2c_8xx_probe(struct of_device *ofdev,
> + const struct of_device_id *match)
> +{
> + int result;
> + struct m8xx_i2c *i2c;
> +
> + i2c = kzalloc(sizeof(*i2c), GFP_KERNEL);
> + if (!i2c)
> + return -ENOMEM;
> +
> + i2c->ofdev = ofdev;
> + i2c->algo_8xx = &m8xx_data;
> +
> + m8xx_iic_init(i2c);
> +
> + dev_set_drvdata(&ofdev->dev, i2c);
> +
> + i2c->adap = m8xx_ops;
> + i2c_set_adapdata(&i2c->adap, i2c);
> + i2c->adap.dev.parent = &ofdev->dev;
> +
> + result = i2c_cpm_add_bus(&i2c->adap);
> + if (result < 0) {
> + printk(KERN_ERR "i2c-8xx: Unable to register with I2C\n");
> + kfree(i2c);
> + }
> +
> + return result;
> +}
> +
> +static int i2c_8xx_remove(struct of_device *ofdev)
> +{
> + struct m8xx_i2c *i2c = dev_get_drvdata(&ofdev->dev);
> +
> + i2c_cpm_del_bus(&i2c->adap);
> + dev_set_drvdata(&ofdev->dev, NULL);
> +
> + kfree(i2c);
> + return 0;
> +}
> +
> +static struct of_device_id i2c_8xx_match[] = {
> + {
> + .type = "i2c",
> + .compatible = "fsl,i2c-cpm",
> + },
> + {},
> +};
> +
Why is an 8xx driver matching all i2c cpm (i.e. what about cpm2)?
-Scot
^ permalink raw reply
* Re: [PATCH 1/5] Update ibm_newemac to use dcr_host_t.base
From: Jeff Garzik @ 2007-10-15 18:34 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <33ee227bae85a8d7ccde69a717cff47000888354.1192440801.git.michael@ellerman.id.au>
Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the
> ibm_newemac code, rather than storing it separately.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> drivers/net/ibm_newemac/mal.c | 9 +++++----
> drivers/net/ibm_newemac/mal.h | 5 ++---
> 2 files changed, 7 insertions(+), 7 deletions(-)
applied 1-5
^ permalink raw reply
* Re: [PATCH v2 3/4] Implement clockevents driver for powerpc
From: Sergei Shtylyov @ 2007-10-15 18:33 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: linuxppc-dev, Thomas Gleixner, Paul Mackerras, Realtime Kernel
In-Reply-To: <4713A616.3090103@ru.mvista.com>
Hello, I wrote:
>>@@ -797,6 +796,53 @@ void __init clocksource_init(void)
>> clock->name, clock->mult, clock->shift);
>> }
>>+static int decrementer_set_next_event(unsigned long evt,
>>+ struct clock_event_device *dev)
>>+{
>>+ set_dec(evt);
> I'd use (evt - 1) since the interrupt gets generated at 0xffffffff count,
> not 0 (on classic CPUs). With you removing of the code that compensated for
> the errors, they will accumulate. And no, this wouldn't be enough anyway,
> since on 40x and Book E you'll need to set it for evt anyway, since the
> interrupt happens at 0 count...
What that means is that the off-by-one-clock drift is introduced for
classic CPUs (not 40x or Book E which interrupt at 0). And this has dealt
with months ago in the clockevent driver in the -rt patch. So much for my
efforts...
> NAK the patch.
It's too late for NAKs -- I should've given these patches more attention
(yet I was in hospital for 1.5 months).
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] PowerPC: Add BCM5248 and Marvell 88E1111 PHY support to NEW EMAC.
From: Jeff Garzik @ 2007-10-15 18:27 UTC (permalink / raw)
To: Valentine Barshak, jwboyer, Michael Ellerman; +Cc: linuxppc-dev, netdev
In-Reply-To: <20071015175717.GA4602@ru.mvista.com>
Valentine Barshak wrote:
> This patch adds BCM5248 and Marvell 88E1111 PHY support to NEW EMAC driver.
> These PHY chips are used on PowerPC 440EPx boards.
> The PHY code is based on the previous work by Stefan Roese <sr@denx.de>
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
> ---
> drivers/net/ibm_newemac/phy.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 39 insertions(+)
>
> --- linux.orig/drivers/net/ibm_newemac/phy.c 2007-06-15 21:45:18.000000000 +0400
> +++ linux/drivers/net/ibm_newemac/phy.c 2007-06-15 20:45:15.000000000 +0400
> @@ -306,8 +306,47 @@
> .ops = &cis8201_phy_ops
> };
>
> +static struct mii_phy_def bcm5248_phy_def = {
> +
> + .phy_id = 0x0143bc00,
> + .phy_id_mask = 0x0ffffff0,
> + .name = "BCM5248 10/100 SMII Ethernet",
> + .ops = &generic_phy_ops
> +};
> +
> +static int m88e1111_init(struct mii_phy *phy)
> +{
> + printk("%s: Marvell 88E1111 Ethernet\n", __FUNCTION__);
> + phy_write(phy, 0x14, 0x0ce3);
> + phy_write(phy, 0x18, 0x4101);
> + phy_write(phy, 0x09, 0x0e00);
> + phy_write(phy, 0x04, 0x01e1);
> + phy_write(phy, 0x00, 0x9140);
> + phy_write(phy, 0x00, 0x1140);
> +
> + return 0;
> +}
> +
> +static struct mii_phy_ops m88e1111_phy_ops = {
> + .init = m88e1111_init,
> + .setup_aneg = genmii_setup_aneg,
> + .setup_forced = genmii_setup_forced,
> + .poll_link = genmii_poll_link,
> + .read_link = genmii_read_link
> +};
> +
> +static struct mii_phy_def m88e1111_phy_def = {
> +
> + .phy_id = 0x01410CC0,
> + .phy_id_mask = 0x0ffffff0,
> + .name = "Marvell 88E1111 Ethernet",
> + .ops = &m88e1111_phy_ops,
> +};
> +
> static struct mii_phy_def *mii_phy_table[] = {
> &cis8201_phy_def,
> + &bcm5248_phy_def,
> + &m88e1111_phy_def,
> &genmii_phy_def,
Seems sane to me -- ACK -- but we have multiple people sending me
patches for a single driver. That's normal for janitorial cleanups
across the whole tree, but discouraged when multiple people are actively
working on the same driver.
Please coordinate, and have ONE person send me patches...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox