LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH-V2] mpc8xxx_gpio: add interrupt support
From: Peter Korsgaard @ 2010-01-04  8:20 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <1261398375-32545-1-git-send-email-jacmet@sunsite.dk>

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

 Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>

Comments?

With the added documentation I believe it's ready to go..

 Peter> ---
 Peter>  .../powerpc/dts-bindings/fsl/8xxx_gpio.txt         |   22 +++-
 Peter>  arch/powerpc/sysdev/mpc8xxx_gpio.c                 |  147 ++++++++++++++++++++
 Peter>  2 files changed, 168 insertions(+), 1 deletions(-)

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* Re: [U-Boot] [PATCH v2 3/3] powerpc: Add support for ram filesystems in FIT uImages
From: Grant Likely @ 2010-01-04  8:27 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: linuxppc-dev, Peter Tyser, linux-kbuild, u-boot
In-Reply-To: <20100103101048.AF4B7E34A26@gemini.denx.de>

On Sun, Jan 3, 2010 at 3:10 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Grant Likely,
>
> In message <fa686aa41001022108i92596d6qdf2da0e24c34767e@mail.gmail.com> y=
ou wrote:
>
> Note that the FIT image can also be made to contain a number of DT
> blobs, and selection of a "board profile" then can be used to boot the
> very sane FIT image file on any of the supported boards - so FIT
> images inherently support multibooting.
>> > I see your point. =A0The main goal of the patch was to introduce FIT i=
mage
>> > support as its the new, more flexible, "better", standard image format
>> > for U-Boot going forward. =A0Also, lots people aren't aware of FIT ima=
ges
>> > and the cool stuff they can do with them, so what better way to get th=
e
>> > word out than getting support for FIT images included in the kernel
>> > proper:)
>>
>> Define 'better'. =A0:-)
>
> FIT images are better than the old uImage format because they:
>
> - allow for strong checksum algorithms like MD5, SHA1, ... (the plain
> =A0CRC32 method is not good enough if you for example want to run
> =A0software in a slot machine in Las Vegas).
>
> - can combine multiple kernel images, device tree blobs and root file
> =A0system images in arbitrary combinations; this allows for example
> =A0for multibooting the same image on different boards by selecting
> =A0the right DTB, for software updates with automatic fall-back, etc.
>
> - can be extended to add new features, images types or whatever in a
> =A0standard way, using a standard technology (device tree support)
> =A0which is already present anyway, i. e. without additional code
> =A0overhead.

That's a lot of variability.  More on this at the end of my email...

>> this regard, and I know that multiplatform is a big deal for some of
>> the users. =A0I'm not interested in encouraging any behaviour that will
>> scuttle deployment of multiplatform kernels. =A0(That being said, I'm
>
> You misunderstand. FIT images do not scuttle multiplatform kernels.
> Instead they support this much better, as they allow to bundle the
> repsective DTBs into one image file.

I believe I do understand.  The problem with this model is that it
makes the assumption that the kernel image builder has access to ahead
of time the full set of bootable platforms.  That assumption doesn't
hold anymore for a lot of markets where many vendors use the same SoC
in slightly different ways.  I think hardware vendors should be
encouraged to manage dtb blobs separately from the kernel so that we
can get out of the current paradigm of only supporting boards we know
about.

Practical example: Ubuntu on ARM netbooks.  Ubuntu is really not
interested in generating new kernel images every time a vendor
releases a new or modified piece of hardware.

If a vendor still decides that they want a kernel+dtb image then fine;
but I don't think it does the Linux ecosystem any favors to make it
the default choice.

>> not going to actively sabotage people who want to put dtb sections
>> into the kernel images either. =A0I understand that at times it is
>> necessary, and there are numerous examples of this already in the
>> kernel tree; specifically to support firmware that doesn't implement
>> arch/powerpc's boot interface)
>
> Even if the boot loader supports it, it is usually pretty benefical
> (especially during development) if you can do this.

Nothing prevents using a custom install script to wrap up a custom fit
image when copying it into the tftp directory (or wherever).

>> I'd be okay (perhaps not happy, but okay) with merging fitImage and
>> fitImage.initrd targets (no dtb). =A0I will resist merging fitImage.%
>> and fitImage.initrd.% targets because I see that very much as a
>> project specific deployment target and I'm not convinced yet that it
>> the pattern is right or that it is even needed in the kernel at all.
>
> Is this just your personal opinion, or do you think that this is
> really what a majority of kernel developers and users are wanting?

I've stated my opinion, my reasons for it, and what I'm willing to
merge.  However, I'm not the only PowerPC maintainer and the others
may disagree.  While I don't particularly like the fit image format,
it won't stop me from merging fitImage and fitImage.initrd patches.

I will not merge fitImage.% and fitImage.initrd.% targets because of
the variability I commented on above.  As you mentioned, there is a
huge amount of flexability in what sections are placed into a fit
image, such as carrying multiple dtb blobs.  However, a fitImage.%
pattern can only express a very specific subset (one kernel image +
one dtb image).  I do not want to codify a particular pattern into the
build system when it seems quite apparent that make targets are
nowhere near expressive enough to do what you're talking about with
fit images.  As is, the targets with embedded .dtbs are already rife
with special cases and other nastiness.

So, I choose to punt it out of the kernel.  If you want to build fit
images beyond just containing an kernel image, then use a custom
install or postprocessing script to assemble the vmlinux, dtb and
initrd building blocks into the image you need.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH v2 3/3] powerpc: Add support for ram filesystems in FIT uImages
From: Grant Likely @ 2010-01-04  8:34 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: linuxppc-dev, Peter Tyser, Wolfgang Denk, linux-kbuild
In-Reply-To: <87skanp8ln.fsf@macbook.be.48ers.dk>

On Sun, Jan 3, 2010 at 7:27 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Wolfgang" =3D=3D Wolfgang Denk <wd@denx.de> writes:
>
> Hi,
>
> =A0>> No, that would break stuff for the existing users. The existing for=
mat
> =A0>> make/file names shouldn't change.
>
> =A0Wolfgang> Well, with this argument you can block all progress and free=
ze all
> =A0Wolfgang> development to some ancient state.
>
> We only break backwards compatibility when we have to. There's no
> technical reason for renaming the existing make targets.

I agree, and I feel pretty strongly on this point.  The meaning of the
uImage target should not change.  The new format must use a different
target name.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: PCI-PCI bridge scanning broken on 460EX
From: Felix Radensky @ 2010-01-04  8:59 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Stefan Roese, Feng Kan
In-Reply-To: <1262584539.2173.335.camel@pasglop>

Hi, Ben

Adding Feng Kan from AMCC to CC.

Benjamin Herrenschmidt wrote:
> On Mon, 2009-12-28 at 12:51 +0200, Felix Radensky wrote:
>   
>> Hi,
>>
>> I'm running linux-2.6.33-rc2 on Canyonlands board. When PLX 6254 
>> transparent PCI-PCI
>> bridge is plugged into PCI slot the kernel simply resets the board 
>> without printing anything
>> to console. Without PLX bridge kernel boots fine.
>>     
>
> Sorry for the late reply...
>   

No need to apologize, I appreciate you help very much.

>   
>> I've tracked down the problem to the following code in pci_scan_bridge() 
>> in drivers/pci/probe.c:
>>
>> if (pcibios_assign_all_busses() || broken)
>>                 /* Temporarily disable forwarding of the
>>                    configuration cycles on all bridges in
>>                    this bus segment to avoid possible
>>                    conflicts in the second pass between two
>>                    bridges programmed with overlapping
>>                    bus ranges. */
>>                 pci_write_config_dword(dev, PCI_PRIMARY_BUS,
>>                                buses & ~0xffffff);
>>
>> If test for broken is removed, kernel boots fine, detects the bridge, but
>> does not detect the device behind the bridge. The same device plugged
>> directly into PCI slot is detected correctly.
>>     
>
> So we would have a similar mismatch between the initial setup and the
> kernel...  However, I don't quite see yet why the kernel trying to fix
> it up breaks things, that will need a bit more debugging here...
>
> Can you give it a quick try with adding something like :
>
>  ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
>
> Near the end of ppc4xx_pci.c ? It looks like another case of reset
> not actually resetting bridges (are we not properly doing a fundamental
> reset ? Stefan what's your take there ?)
>
> The above will cause busses to be re-assigned which is risky because it
> will allow the kernel to assign numbers beyond the limits of what
> ppc4xx_pci.c supports (see my comments in the thread you quotes).
>
> The good thing is that we now have a working fixmap infrastructure, so
> we could/should just move ppc4xx_pci.c to use that, and just always
> re-assign busses.
>
>   
>> To remind you, tests for broken were added by commit 
>> a1c19894b786f10c76ac40e93c6b5d70c9b946d2,
>> and were intended to solve device detection problem behind PCI-E 
>> switches, as discussed in this thread:
>> http://lists.ozlabs.org/pipermail/linuxppc-dev/2008-October/063939.html
>>     
>
>   
>> PCI: Probing PCI hardware
>> pci_bus 0000:00: scanning bus
>> pci 0000:00:06.0: found [3388:0020] class 000604 header type 01
>> pci 0000:00:06.0: supports D1 D2
>> pci 0000:00:06.0: PME# supported from D0 D1 D2 D3hot
>> pci 0000:00:06.0: PME# disabled
>> pci_bus 0000:00: fixups for bus
>> pci 0000:00:06.0: scanning behind bridge, config 000000, pass 0
>> pci 0000:00:06.0: bus configuration invalid, reconfiguring
>>     
>
> Ok so we hit a P2P bridge whose primary, secondary and subordinate bus
> numbers are all 0, which is clearly unconfigured. I think this is the
> root complex bridge
>
>   
>> pci 0000:00:06.0: scanning behind bridge, config 000000, pass 1
>>     
>
> Now this is when the bus should be reconfigured (pass 1). Sadly the code
> doesn't print much debug.
>
> Also from that point, it should renumber things and work... 
>
>   
>> pci_bus 0000:01: scanning bus
>>     
>
> Which it does to some extent. It assigned bus number 1 to it afaik so we
> now start looking below the RC bridge:
>
>   
>> pci 0000:01:06.0: found [3388:0020] class 000604 header type 01
>>     
>
> Hrm... class PCI bridge, vendor 3388 device 0020, is that your PLX ?
> It's not the right vendor ID but maybe that's configurable by our OEM or
> something...
>   

The bridge and its evaluation board were manufactured by HiNT, later 
purchased by PLX.
3388:0020 is HiNT HB6 Universal PCI-PCI bridge in transparent mode.

>   
>> pci 0000:01:06.0: supports D1 D2
>> pci 0000:01:06.0: PME# supported from D0 D1 D2 D3hot
>> pci 0000:01:06.0: PME# disabled
>> pci_bus 0000:01: fixups for bus
>> pci 0000:00:06.0: PCI bridge to [bus 01-ff]
>> pci 0000:00:06.0:   bridge window [io  0x0000-0x0fff]
>> pci 0000:00:06.0:   bridge window [mem 0x00000000-0x000fffff]
>> pci 0000:00:06.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
>> pci 0000:01:06.0: scanning behind bridge, config ff0100, pass 0
>>     
>
> Allright, that's where it gets interesting. It tries to scan behind the
> bridge. It gets something it doesn't like. IE, it gets a secondary bus
> number of 1 (what the heck ? I wonder what your firmware does) which
> Linux is not happy about and decides to renumber it.
>   

U-boot has problems with this bridge as well, so I had to completely 
disable PCI
support in u-boot to get linux running.
>   
>> pci 0000:01:06.0: bus configuration invalid, reconfiguring
>>     
>
> Now, that's where Linux should have written 000000 to the register,
> which is what you commented out.
>
>   
>> pci 0000:01:06.0: scanning behind bridge, config ff0100, pass 1
>> pci_bus 0000:01: bus scan returning with max=01
>> pci_bus 0000:00: bus scan returning with max=01
>>     
>
> Because of that commenting out, it doesn't see the config as 000000 and
> thus doesn't re-assign a bus number in pass 1, so from there you can't
> see what's behind the bus.
>
> So we have two things here:
>
>  - It seems like the writing of 000000 to the register in pass 0 is
> causing your crash. Can you verify that ? IE. Can you verify that it's
> indeed crashing on this specific statement:
>
> pci_write_config_dword(dev, PCI_PRIMARY_BUS,
>                                buses & ~0xffffff);
>
> When writing to the bridge, and that this seems to be causing a hard
> reboot of the system ?
>   

Yes, this particular statement causes hard reboot. With original broken 
tests restored
and writing to bridge commented out the system boots. If writing to 
bridge happens
I get hard reset.

> It might be useful to ask AMCC how that is possible in HW, ie what kind
> of signal can be causing that. IE, even if the bridge is causing a PCIe
> error, that should not cause a reboot ... right ?
>   

Feng, can you please comment on this ?
>  - You can test a quick hack workaround which consists of changing:
>
> 	/* Check if setup is sensible at all */
> -	if (!pass &&
> -	if (1 &&
> 	    ((buses & 0xff) != bus->number || ((buses >> 8) & 0xff) <= bus->number)) {
> 		dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
> 		broken = 1;
> 	}
>
> In -addition- to your commenting out of the broken test. This will cause the
> second pass to go through the re-assign code path despite the fact that you
> have not written 000000 to the bus numbers.
>   

With this change and commented out broken test I still get hard reset.

I didn't try adding ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS)
If you still want me to try this, please let me know. Should I leave broken
tests enabled in that case ?

Thanks a lot for your help.

Felix.

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfgang Grandegger @ 2010-01-04 12:49 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Socketcan-core, Netdev, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <4B3F6F6B.5070004@grandegger.com>

Wolfgang Grandegger wrote:
> Wolfram Sang wrote:
>> On Sat, Jan 02, 2010 at 09:17:53AM +0100, Wolfgang Grandegger wrote:
>>> From: Wolfgang Grandegger <wg@denx.de>
>>>
>>> The main differences compared to the MSCAN on the MPC5200 are:
>>>
>>> - More flexibility in choosing the CAN source clock and frequency:
>>>
>>>   Three different clock sources can be selected: "ip", "ref" or "sys".
>>>   For the latter two, a clock divider can be defined as well. If the
>>>   clock source is not specified by the device tree, we first try to
>>>   find an optimal CAN source clock based on the system clock. If that
>>>   is not possible, the reference clock will be used.
>>>
>>> - The behavior of bus-off recovery is configurable:
>>>
>>>   To comply with the usual handling of Socket-CAN bus-off recovery,
>>>   "recovery on request" is selected (instead of automatic recovery).
>>>
>>> Signed-off-by: Wolfgang Grandegger <wg@denx.de>
>>> ---
>>>  drivers/net/can/mscan/Kconfig       |    2 +-
>>>  drivers/net/can/mscan/mpc5xxx_can.c |  234 +++++++++++++++++++++++++++++------
>>>  drivers/net/can/mscan/mscan.c       |   41 +++++--
>>>  drivers/net/can/mscan/mscan.h       |   81 ++++++------
>>>  4 files changed, 271 insertions(+), 87 deletions(-)
>>>
> [snip]
> 
>>> +#else /* !CONFIG_PPC_MPC5200 */
>>> +static u32 __devinit mpc52xx_can_get_clock(struct of_device *ofdev,
>>> +					   const char *clock_name,
>>> +					   int *mscan_clksrc)
>>> +{
>>> +	return 0;
>>> +}
>>> +#endif /* CONFIG_PPC_MPC5200 */
>> Hmmm, I don't really like those empty functions. I once used the data-field of
>> struct of_device_id, which carried a function pointer to a specific
>> init-function for the matched device. What do you think about such an approach?
> 
> Often the problem is that the function will not compile on the other MPC
> arch. This is not true here. So, the main reason for the #ifdefs is
> space saving. Your approach will not help in both cases.
> 
>>> +
>>> +#ifdef CONFIG_PPC_MPC512x
>>> +struct mpc512x_clockctl {
>>> +	u32 spmr;		/* System PLL Mode Reg */
>>> +	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
>>> +	u32 scfr1;		/* System Clk Freq Reg 1 */
>>> +	u32 scfr2;		/* System Clk Freq Reg 2 */
>>> +	u32 reserved;
>>> +	u32 bcr;		/* Bread Crumb Reg */
>>> +	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
>>> +	u32 spccr;		/* SPDIF Clk Ctrl Reg */
>>> +	u32 cccr;		/* CFM Clk Ctrl Reg */
>>> +	u32 dccr;		/* DIU Clk Cnfg Reg */
>>> +	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
>>> +};
>>> +
>>> +static struct of_device_id mpc512x_clock_ids[] __devinitdata = {
>>> +	{ .compatible = "fsl,mpc5121-clock", },
>>> +	{}
>>> +};
>>> +
>>> +static u32  __devinit mpc512x_can_get_clock(struct of_device *ofdev,
>>> +					    const char *clock_name,
>>> +					    int *mscan_clksrc,
>>> +					    ssize_t mscan_addr)
>>> +{
>>> +	struct mpc512x_clockctl __iomem *clockctl;
>>> +	struct device_node *np_clock;
>>> +	struct clk *sys_clk, *ref_clk;
>>> +	int plen, clockidx, clocksrc = -1;
>>> +	u32 sys_freq, val, clockdiv = 1, freq = 0;
>>> +	const u32 *pval;
>>> +
>>> +	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
>>> +	if (!np_clock) {
>>> +		dev_err(&ofdev->dev, "couldn't find clock node\n");
>>> +		return -ENODEV;
>>> +	}
>>> +	clockctl = of_iomap(np_clock, 0);
>>> +	if (!clockctl) {
>>> +		dev_err(&ofdev->dev, "couldn't map clock registers\n");
>>> +		return 0;
>>> +	}
>>> +
>>> +	/* Determine the MSCAN device index from the physical address */
>>> +	clockidx = (mscan_addr & 0x80) ? 1 : 0;
>>> +	if (mscan_addr & 0x2000)
>>> +		clockidx += 2;
>> The PSCs use 'cell-index', here we use mscan_addr to derive the index. This is
>> not consistent, but should be IMHO. Now, which is the preferred way? I think
>> I'd go for 'cell-index', as other processors might have mscan_addr shuffled.
>> Also, we could use 'of_iomap' again in the probe_routine.
> 
> I understood that "cell-index" is deprecated and it has been removed
> from many nodes. That's why I used the address to derive the index.

So more thoughts: I still find inspecting the regs less error prune than
defining cell-index and it should work fine for "fsl,mpc5121_mscan".
Other processor variants might handle a different register layout with
another appropriate compatibility string. But I could retrieve the
"regs" property inside mpc512x_can_get_clock() to use of_iomap() as before.

Wolfgang.

^ permalink raw reply

* Please pull 'merge' branch of 4xx tree
From: Josh Boyer @ 2010-01-04 16:13 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

Hi Ben,

Below are some simple defconfig updates for 4xx for 2.6.33.  Please pull them
before you send the next batch to Linus.

Thanks,
josh

The following changes since commit 95cd34b42b43c0ed5a89a764e023189bfe7b1530:
  Albert Herranz (1):
        powerpc/gc/wii: Remove get_irq_desc()

are available in the git repository at:

  ssh://master.kernel.org/pub/scm/linux/kernel/git/jwboyer/powerpc-4xx.git merge

Josh Boyer (2):
      powerpc/44x: Update PowerPC 44x board defconfigs
      powerpc/40x: Update the PowerPC 40x board defconfigs

 arch/powerpc/configs/40x/acadia_defconfig      |  241 ++++++++++++++-----
 arch/powerpc/configs/40x/ep405_defconfig       |  307 ++++++++++++++++++------
 arch/powerpc/configs/40x/hcu4_defconfig        |  256 +++++++++++++++-----
 arch/powerpc/configs/40x/kilauea_defconfig     |  140 ++++++++---
 arch/powerpc/configs/40x/makalu_defconfig      |  190 ++++++++++-----
 arch/powerpc/configs/40x/walnut_defconfig      |  257 +++++++++++++++-----
 arch/powerpc/configs/44x/arches_defconfig      |  135 ++++++++---
 arch/powerpc/configs/44x/bamboo_defconfig      |  255 +++++++++++++++-----
 arch/powerpc/configs/44x/canyonlands_defconfig |  141 ++++++++---
 arch/powerpc/configs/44x/ebony_defconfig       |  258 +++++++++++++++-----
 arch/powerpc/configs/44x/eiger_defconfig       |  128 +++++++---
 arch/powerpc/configs/44x/katmai_defconfig      |  253 +++++++++++++++-----
 arch/powerpc/configs/44x/rainier_defconfig     |  254 +++++++++++++++-----
 arch/powerpc/configs/44x/redwood_defconfig     |  274 +++++++++++++++------
 arch/powerpc/configs/44x/sam440ep_defconfig    |  284 +++++++++++++++-------
 arch/powerpc/configs/44x/sequoia_defconfig     |  202 +++++++++++-----
 arch/powerpc/configs/44x/taishan_defconfig     |  258 +++++++++++++++-----
 arch/powerpc/configs/44x/warp_defconfig        |  228 ++++++++++++------
 arch/powerpc/configs/ppc40x_defconfig          |  292 ++++++++++++++++-------
 arch/powerpc/configs/ppc44x_defconfig          |  309 +++++++++++++++++-------
 20 files changed, 3391 insertions(+), 1271 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfram Sang @ 2010-01-04 16:24 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Socketcan-core, Netdev, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <4B3F6F6B.5070004@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 4101 bytes --]

Hello Wolfgang,

first the good news: Your patches also work with our MPC5121-board.

> >> +#else /* !CONFIG_PPC_MPC5200 */
> >> +static u32 __devinit mpc52xx_can_get_clock(struct of_device *ofdev,
> >> +					   const char *clock_name,
> >> +					   int *mscan_clksrc)
> >> +{
> >> +	return 0;
> >> +}
> >> +#endif /* CONFIG_PPC_MPC5200 */
> > 
> > Hmmm, I don't really like those empty functions. I once used the data-field of
> > struct of_device_id, which carried a function pointer to a specific
> > init-function for the matched device. What do you think about such an approach?
> 
> Often the problem is that the function will not compile on the other MPC
> arch. This is not true here. So, the main reason for the #ifdefs is
> space saving. Your approach will not help in both cases.

My idea was: it might be nice to save both #else-branches and the if-clause in
probe() which calls this get_clock() or the other (and then another in case
there will be a new mpc5xyz-user in the future). And replace it with some
mpc5xxx_custom_init() which is taken from of_device_id->data. No big issue,
though; no show-stopper.

> >> +
> >> +#ifdef CONFIG_PPC_MPC512x
> >> +struct mpc512x_clockctl {
> >> +	u32 spmr;		/* System PLL Mode Reg */
> >> +	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
> >> +	u32 scfr1;		/* System Clk Freq Reg 1 */
> >> +	u32 scfr2;		/* System Clk Freq Reg 2 */
> >> +	u32 reserved;
> >> +	u32 bcr;		/* Bread Crumb Reg */
> >> +	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
> >> +	u32 spccr;		/* SPDIF Clk Ctrl Reg */
> >> +	u32 cccr;		/* CFM Clk Ctrl Reg */
> >> +	u32 dccr;		/* DIU Clk Cnfg Reg */
> >> +	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
> >> +};

I wonder if this (and the occurence in clock.c) should be factored out and
moved to asm/mpc5xxx.h?

> >> +
> >> +static struct of_device_id mpc512x_clock_ids[] __devinitdata = {
> >> +	{ .compatible = "fsl,mpc5121-clock", },
> >> +	{}
> >> +};
> >> +
> >> +static u32  __devinit mpc512x_can_get_clock(struct of_device *ofdev,
> >> +					    const char *clock_name,
> >> +					    int *mscan_clksrc,
> >> +					    ssize_t mscan_addr)
> >> +{
> >> +	struct mpc512x_clockctl __iomem *clockctl;
> >> +	struct device_node *np_clock;
> >> +	struct clk *sys_clk, *ref_clk;
> >> +	int plen, clockidx, clocksrc = -1;
> >> +	u32 sys_freq, val, clockdiv = 1, freq = 0;
> >> +	const u32 *pval;
> >> +
> >> +	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
> >> +	if (!np_clock) {
> >> +		dev_err(&ofdev->dev, "couldn't find clock node\n");
> >> +		return -ENODEV;
> >> +	}
> >> +	clockctl = of_iomap(np_clock, 0);
> >> +	if (!clockctl) {
> >> +		dev_err(&ofdev->dev, "couldn't map clock registers\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	/* Determine the MSCAN device index from the physical address */
> >> +	clockidx = (mscan_addr & 0x80) ? 1 : 0;
> >> +	if (mscan_addr & 0x2000)
> >> +		clockidx += 2;
> > 
> > The PSCs use 'cell-index', here we use mscan_addr to derive the index. This is
> > not consistent, but should be IMHO. Now, which is the preferred way? I think
> > I'd go for 'cell-index', as other processors might have mscan_addr shuffled.
> > Also, we could use 'of_iomap' again in the probe_routine.
> 
> I understood that "cell-index" is deprecated and it has been removed
> from many nodes. That's why I used the address to derive the index.

Well, the arguments in your other mail make sense to me, so keep it this way.
As not only the index-issue, but also the clock-handling is different for the
PSCs, it is at least consistently inconsistent :D

One further note: I couldn't spot any code handling Rev1 of the MPC5121? Do you
plan to add such code? If not, we should at least put a comment that it is
missing. The binding documentation should be updated as well, as you can't use
all options on such revisions.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfgang Grandegger @ 2010-01-04 16:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Socketcan-core, Netdev, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <20100104162441.GA4665@pengutronix.de>

Hi Wolfram,

Wolfram Sang wrote:
> Hello Wolfgang,
> 
> first the good news: Your patches also work with our MPC5121-board.

Nice. Just for curiosity, what clock and frequency does it select on
your board? It should be listed when the driver is loaded.

>>>> +#else /* !CONFIG_PPC_MPC5200 */
>>>> +static u32 __devinit mpc52xx_can_get_clock(struct of_device *ofdev,
>>>> +					   const char *clock_name,
>>>> +					   int *mscan_clksrc)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +#endif /* CONFIG_PPC_MPC5200 */
>>> Hmmm, I don't really like those empty functions. I once used the data-field of
>>> struct of_device_id, which carried a function pointer to a specific
>>> init-function for the matched device. What do you think about such an approach?
>> Often the problem is that the function will not compile on the other MPC
>> arch. This is not true here. So, the main reason for the #ifdefs is
>> space saving. Your approach will not help in both cases.
> 
> My idea was: it might be nice to save both #else-branches and the if-clause in
> probe() which calls this get_clock() or the other (and then another in case
> there will be a new mpc5xyz-user in the future). And replace it with some
> mpc5xxx_custom_init() which is taken from of_device_id->data. No big issue,
> though; no show-stopper.

You mean like in the i2c-mpc driver:

http://lxr.linux.no/#linux+v2.6.32/drivers/i2c/busses/i2c-mpc.c#L585

No problem, if I handle the regs property inside the mpc5121-specific
function. The #ifdef's are for *space saving*. If nobody else than me
cares, I will remove them.

>>>> +
>>>> +#ifdef CONFIG_PPC_MPC512x
>>>> +struct mpc512x_clockctl {
>>>> +	u32 spmr;		/* System PLL Mode Reg */
>>>> +	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
>>>> +	u32 scfr1;		/* System Clk Freq Reg 1 */
>>>> +	u32 scfr2;		/* System Clk Freq Reg 2 */
>>>> +	u32 reserved;
>>>> +	u32 bcr;		/* Bread Crumb Reg */
>>>> +	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
>>>> +	u32 spccr;		/* SPDIF Clk Ctrl Reg */
>>>> +	u32 cccr;		/* CFM Clk Ctrl Reg */
>>>> +	u32 dccr;		/* DIU Clk Cnfg Reg */
>>>> +	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
>>>> +};
> 
> I wonder if this (and the occurence in clock.c) should be factored out and
> moved to asm/mpc5xxx.h?

I was thinking about that as well but mpc5xxx.h seems not (yet) to be a
popular place to store mpc5xxx related definitions.

>>>> +
>>>> +static struct of_device_id mpc512x_clock_ids[] __devinitdata = {
>>>> +	{ .compatible = "fsl,mpc5121-clock", },
>>>> +	{}
>>>> +};
>>>> +
>>>> +static u32  __devinit mpc512x_can_get_clock(struct of_device *ofdev,
>>>> +					    const char *clock_name,
>>>> +					    int *mscan_clksrc,
>>>> +					    ssize_t mscan_addr)
>>>> +{
>>>> +	struct mpc512x_clockctl __iomem *clockctl;
>>>> +	struct device_node *np_clock;
>>>> +	struct clk *sys_clk, *ref_clk;
>>>> +	int plen, clockidx, clocksrc = -1;
>>>> +	u32 sys_freq, val, clockdiv = 1, freq = 0;
>>>> +	const u32 *pval;
>>>> +
>>>> +	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
>>>> +	if (!np_clock) {
>>>> +		dev_err(&ofdev->dev, "couldn't find clock node\n");
>>>> +		return -ENODEV;
>>>> +	}
>>>> +	clockctl = of_iomap(np_clock, 0);
>>>> +	if (!clockctl) {
>>>> +		dev_err(&ofdev->dev, "couldn't map clock registers\n");
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	/* Determine the MSCAN device index from the physical address */
>>>> +	clockidx = (mscan_addr & 0x80) ? 1 : 0;
>>>> +	if (mscan_addr & 0x2000)
>>>> +		clockidx += 2;
>>> The PSCs use 'cell-index', here we use mscan_addr to derive the index. This is
>>> not consistent, but should be IMHO. Now, which is the preferred way? I think
>>> I'd go for 'cell-index', as other processors might have mscan_addr shuffled.
>>> Also, we could use 'of_iomap' again in the probe_routine.
>> I understood that "cell-index" is deprecated and it has been removed
>> from many nodes. That's why I used the address to derive the index.
> 
> Well, the arguments in your other mail make sense to me, so keep it this way.
> As not only the index-issue, but also the clock-handling is different for the
> PSCs, it is at least consistently inconsistent :D

OK.

> One further note: I couldn't spot any code handling Rev1 of the MPC5121? Do you
> plan to add such code? If not, we should at least put a comment that it is
> missing. The binding documentation should be updated as well, as you can't use
> all options on such revisions.

Do we have rev1 support in the mainline kernel? I also understood that
there are only a few devel boards out there with v1 CPUs. If necessary,
this could be fixed later on demand. But it should be documented, e.g.
in the KConfig and dts bindings doc, of course.

Did you have a chance to test bus-off recovery? I just realized one
issue if the device is stopped while in bus-off.

Will come up with a v2 patch soon...

Thanks,

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfram Sang @ 2010-01-04 17:22 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Socketcan-core, Netdev, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <4B421CDB.4010902@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 3693 bytes --]


> Nice. Just for curiosity, what clock and frequency does it select on
> your board? It should be listed when the driver is loaded.

Using this simple dts-snipplet

                mscan@1300 {
                        compatible = "fsl,mpc5121-mscan";
                        cell-index = <0>;
                        interrupts = <12 8>;
                        interrupt-parent = < &ipic >;
                        reg = <0x1300 0x80>;
                };

I get this output:

mpc5xxx_can 80001300.mscan: using 'sys_clk' with frequency divider 25
mpc5xxx_can 80001300.mscan: MSCAN at 0xe1064300, irq 16, clock 16000000 Hz

mpc5xxx_can 80001380.mscan: using 'sys_clk' with frequency divider 25
mpc5xxx_can 80001380.mscan: MSCAN at 0xe106c380, irq 17, clock 16000000 Hz


> > My idea was: it might be nice to save both #else-branches and the if-clause in
> > probe() which calls this get_clock() or the other (and then another in case
> > there will be a new mpc5xyz-user in the future). And replace it with some
> > mpc5xxx_custom_init() which is taken from of_device_id->data. No big issue,
> > though; no show-stopper.
> 
> You mean like in the i2c-mpc driver:
> 
> http://lxr.linux.no/#linux+v2.6.32/drivers/i2c/busses/i2c-mpc.c#L585

Yes.

> No problem, if I handle the regs property inside the mpc5121-specific
> function. The #ifdef's are for *space saving*. If nobody else than me
> cares, I will remove them.

I'd be fine with keeping them.

> >>>> +
> >>>> +#ifdef CONFIG_PPC_MPC512x
> >>>> +struct mpc512x_clockctl {
> >>>> +	u32 spmr;		/* System PLL Mode Reg */
> >>>> +	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
> >>>> +	u32 scfr1;		/* System Clk Freq Reg 1 */
> >>>> +	u32 scfr2;		/* System Clk Freq Reg 2 */
> >>>> +	u32 reserved;
> >>>> +	u32 bcr;		/* Bread Crumb Reg */
> >>>> +	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
> >>>> +	u32 spccr;		/* SPDIF Clk Ctrl Reg */
> >>>> +	u32 cccr;		/* CFM Clk Ctrl Reg */
> >>>> +	u32 dccr;		/* DIU Clk Cnfg Reg */
> >>>> +	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
> >>>> +};
> > 
> > I wonder if this (and the occurence in clock.c) should be factored out and
> > moved to asm/mpc5xxx.h?
> 
> I was thinking about that as well but mpc5xxx.h seems not (yet) to be a
> popular place to store mpc5xxx related definitions.

Probably because the mpc5121 is not very popular when it comes to mainline
activity. (BTW, I also wondered why it is not named mpc512x.h as I'd think
mpc5xxx is for common stuff between mpc5200 and mpc5121. But this is another
issue, not relevant here.)

> > One further note: I couldn't spot any code handling Rev1 of the MPC5121? Do you
> > plan to add such code? If not, we should at least put a comment that it is
> > missing. The binding documentation should be updated as well, as you can't use
> > all options on such revisions.
> 
> Do we have rev1 support in the mainline kernel? I also understood that
> there are only a few devel boards out there with v1 CPUs. If necessary,
> this could be fixed later on demand. But it should be documented, e.g.
> in the KConfig and dts bindings doc, of course.

Yup, documenting it will do.

> Did you have a chance to test bus-off recovery? I just realized one
> issue if the device is stopped while in bus-off.

Sorry, I just did basic transfer-tests as I am currently busy with other
projects.

> Will come up with a v2 patch soon...

Please do an s/latetr/latter/ before posting it :)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfgang Grandegger @ 2010-01-04 17:35 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Socketcan-core, Netdev, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <20100104172209.GC4665@pengutronix.de>

Wolfram Sang wrote:
>> Nice. Just for curiosity, what clock and frequency does it select on
>> your board? It should be listed when the driver is loaded.
> 
> Using this simple dts-snipplet
> 
>                 mscan@1300 {
>                         compatible = "fsl,mpc5121-mscan";
>                         cell-index = <0>;
>                         interrupts = <12 8>;
>                         interrupt-parent = < &ipic >;
>                         reg = <0x1300 0x80>;
>                 };
> 
> I get this output:
> 
> mpc5xxx_can 80001300.mscan: using 'sys_clk' with frequency divider 25
> mpc5xxx_can 80001300.mscan: MSCAN at 0xe1064300, irq 16, clock 16000000 Hz
> 
> mpc5xxx_can 80001380.mscan: using 'sys_clk' with frequency divider 25
> mpc5xxx_can 80001380.mscan: MSCAN at 0xe106c380, irq 17, clock 16000000 Hz

OK, the the board uses an oscillator clock of 33.333333 MHz. The "ref"
or "ip" clock would be a worse choice.

>>> My idea was: it might be nice to save both #else-branches and the if-clause in
>>> probe() which calls this get_clock() or the other (and then another in case
>>> there will be a new mpc5xyz-user in the future). And replace it with some
>>> mpc5xxx_custom_init() which is taken from of_device_id->data. No big issue,
>>> though; no show-stopper.
>> You mean like in the i2c-mpc driver:
>>
>> http://lxr.linux.no/#linux+v2.6.32/drivers/i2c/busses/i2c-mpc.c#L585
> 
> Yes.
> 
>> No problem, if I handle the regs property inside the mpc5121-specific
>> function. The #ifdef's are for *space saving*. If nobody else than me
>> cares, I will remove them.
> 
> I'd be fine with keeping them.

OK.

>>>>>> +
>>>>>> +#ifdef CONFIG_PPC_MPC512x
>>>>>> +struct mpc512x_clockctl {
>>>>>> +	u32 spmr;		/* System PLL Mode Reg */
>>>>>> +	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
>>>>>> +	u32 scfr1;		/* System Clk Freq Reg 1 */
>>>>>> +	u32 scfr2;		/* System Clk Freq Reg 2 */
>>>>>> +	u32 reserved;
>>>>>> +	u32 bcr;		/* Bread Crumb Reg */
>>>>>> +	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
>>>>>> +	u32 spccr;		/* SPDIF Clk Ctrl Reg */
>>>>>> +	u32 cccr;		/* CFM Clk Ctrl Reg */
>>>>>> +	u32 dccr;		/* DIU Clk Cnfg Reg */
>>>>>> +	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
>>>>>> +};
>>> I wonder if this (and the occurence in clock.c) should be factored out and
>>> moved to asm/mpc5xxx.h?
>> I was thinking about that as well but mpc5xxx.h seems not (yet) to be a
>> popular place to store mpc5xxx related definitions.
> 
> Probably because the mpc5121 is not very popular when it comes to mainline
> activity. (BTW, I also wondered why it is not named mpc512x.h as I'd think
> mpc5xxx is for common stuff between mpc5200 and mpc5121. But this is another
> issue, not relevant here.)

Well, yes. The mpc512x Socket-CAN mainline support is already in a much
better shape than the base port. I will keep it until the mpc512x port
has matured/settled.

>>> One further note: I couldn't spot any code handling Rev1 of the MPC5121? Do you
>>> plan to add such code? If not, we should at least put a comment that it is
>>> missing. The binding documentation should be updated as well, as you can't use
>>> all options on such revisions.
>> Do we have rev1 support in the mainline kernel? I also understood that
>> there are only a few devel boards out there with v1 CPUs. If necessary,
>> this could be fixed later on demand. But it should be documented, e.g.
>> in the KConfig and dts bindings doc, of course.
> 
> Yup, documenting it will do.
> 
>> Did you have a chance to test bus-off recovery? I just realized one
>> issue if the device is stopped while in bus-off.
> 
> Sorry, I just did basic transfer-tests as I am currently busy with other
> projects.

You are welcome.

>> Will come up with a v2 patch soon...
> 
> Please do an s/latetr/latter/ before posting it :)

OK.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfgang Denk @ 2010-01-04 19:40 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Netdev, Devicetree-discuss, Socketcan-core, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <20100104162441.GA4665@pengutronix.de>

Dear Wolfram,

in message <20100104162441.GA4665@pengutronix.de> you wrote:
> 
> One further note: I couldn't spot any code handling Rev1 of the MPC5121? Do you
> plan to add such code? If not, we should at least put a comment that it is
> missing. The binding documentation should be updated as well, as you can't use
> all options on such revisions.

We only support MPC512x Rev. 2 (and later). This is also true for
other parts of the mainline kernel code - see for example trhe NAND
driver.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Include the success of others in your dreams for your own success.

^ permalink raw reply

* Re: [PATCH net-next 2/3] can: mscan-mpc5xxx: add support for the MPC521x processor
From: Wolfram Sang @ 2010-01-04 20:23 UTC (permalink / raw)
  To: Wolfgang Denk
  Cc: Netdev, Devicetree-discuss, Socketcan-core, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <20100104194027.CB2053F6FD@gemini.denx.de>

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

Hello Wolfgang,

> We only support MPC512x Rev. 2 (and later). This is also true for
> other parts of the mainline kernel code - see for example trhe NAND
> driver.

Okay, good to know there are differences in NAND as well. As said before, it
makes sense to me to skip V1. It just should be documented IMHO, and, if
possible, the bindings should allow V1 support in case anyone dares.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [PATCH] powerpc/pseries: Fix xics interrupt affinity
From: Anton Blanchard @ 2010-01-05  1:26 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev


Commit 57b150cce8e004ddd36330490a68bfb59b7271e9 (irq: only update affinity if
->set_affinity() is sucessfull) broke xics irq affinity.

We need to use the cpumask passed in, instead of accessing ->affinity directly.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

This code needs a rework and a conversion to the new cpumask API,
but I will leave that for subsequent patches.

Index: linux-cpumask/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-cpumask.orig/arch/powerpc/platforms/pseries/xics.c	2010-01-05 11:06:55.658914849 +1100
+++ linux-cpumask/arch/powerpc/platforms/pseries/xics.c	2010-01-05 11:58:42.288311829 +1100
@@ -163,14 +163,13 @@ static inline void lpar_qirr_info(int n_
 /* Interface to generic irq subsystem */
 
 #ifdef CONFIG_SMP
-static int get_irq_server(unsigned int virq, unsigned int strict_check)
+static int get_irq_server(unsigned int virq, cpumask_t cpumask,
+			  unsigned int strict_check)
 {
 	int server;
 	/* For the moment only implement delivery to all cpus or one cpu */
-	cpumask_t cpumask;
 	cpumask_t tmp = CPU_MASK_NONE;
 
-	cpumask_copy(&cpumask, irq_to_desc(virq)->affinity);
 	if (!distribute_irqs)
 		return default_server;
 
@@ -192,7 +191,8 @@ static int get_irq_server(unsigned int v
 	return default_server;
 }
 #else
-static int get_irq_server(unsigned int virq, unsigned int strict_check)
+static int get_irq_server(unsigned int virq, cpumask_t cpumask,
+			  unsigned int strict_check)
 {
 	return default_server;
 }
@@ -211,7 +211,7 @@ static void xics_unmask_irq(unsigned int
 	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
 		return;
 
-	server = get_irq_server(virq, 0);
+	server = get_irq_server(virq, *(irq_to_desc(virq)->affinity), 0);
 
 	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, irq, server,
 				DEFAULT_PRIORITY);
@@ -405,7 +405,7 @@ static int xics_set_affinity(unsigned in
 	 * For the moment only implement delivery to all cpus or one cpu.
 	 * Get current irq_server for the given irq
 	 */
-	irq_server = get_irq_server(virq, 1);
+	irq_server = get_irq_server(virq, *cpumask, 1);
 	if (irq_server == -1) {
 		char cpulist[128];
 		cpumask_scnprintf(cpulist, sizeof(cpulist), cpumask);

^ permalink raw reply

* powerpc: allmod config build failure
From: Stephen Rothwell @ 2010-01-05  3:32 UTC (permalink / raw)
  To: Hollis Blanchard, Benjamin Herrenschmidt; +Cc: ppc-dev, Alexander Graf

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

Hi all,

The powerpc allmodconfig build of Linus' tree has been failing like this
since Dec 13 (and linux-next since Nov 7):

arch/powerpc/kvm/built-in.o:(.toc1+0x18): undefined reference to `kvm_debugfs_dir'

$ grep KVM .config
CONFIG_KVM=y
CONFIG_KVM_BOOK3S_64_HANDLER=y
CONFIG_KVM_BOOK3S_64=m
CONFIG_KVM_EXIT_TIMING=y

I assume that this failure was introduced by commit
c4f9c779f1019c7d5c61961a7c0aaaf1420d8f90 ("Include Book3s_64 target in
buildsystem").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* RE: fsldma: cleanup driver and fix async_tx compatibility
From: Dudhat Dipen-B09055 @ 2010-01-05  6:08 UTC (permalink / raw)
  To: Ira W. Snyder, dan.j.williams
  Cc: herbert, Suresh Vishnu-B05022, Tabi Timur-B04825, linuxppc-dev,
	Gupta Maneesh-B18878, Li Yang-R58472
In-Reply-To: <1262326246-936-1-git-send-email-iws@ovro.caltech.edu>


Hi Ira,

I will test it on 85xx hardware and let you know once done.

Thanks
Dipen
=20

-----Original Message-----
From: Ira W. Snyder [mailto:iws@ovro.caltech.edu]=20
Sent: Friday, January 01, 2010 11:41 AM
To: dan.j.williams@intel.com
Cc: galak@kernel.crashing.org; herbert@gondor.apana.org.au; Tabi
Timur-B04825; linuxppc-dev@ozlabs.org; Suresh Vishnu-B05022; Dudhat
Dipen-B09055; Gupta Maneesh-B18878; Li Yang-R58472
Subject: fsldma: cleanup driver and fix async_tx compatibility

This patch series cleans up the Freescale DMAEngine driver, including
verifying the locking and making sure that all code paths are correct.
There were a few places that seemed suspicious, and they have been
fixed.

I have written a quick memory->memory DMAEngine test driver, and the
performance is identical before and after my changes (<0.1% change). I
measured both setting up the DMA operation (via
device_prep_dma_interrupt() and device_prep_dma_memcpy()) and the actual
DMA transfer itself.

As an added bonus, the interrupt load is measurably reduced. My test
driver transfers 32MB as 32x 1MB chunks + 1 interrupt descriptor, using
the functions noted above. Previous to this patch series, 31 interrupts
were generated. After this patch series, only a single interrupt is
generated for the whole transaction.

Some testing on 85xx/86xx hardware would be appreciated. Also, some
testing by the users attempting to use async_tx and talitos to handle
RAID offload would be great as well.

 Documentation/powerpc/dts-bindings/fsl/dma.txt |   17 +-
 drivers/dma/fsldma.c                           | 1036
++++++++++++------------
 drivers/dma/fsldma.h                           |   35 +-
 3 files changed, 556 insertions(+), 532 deletions(-)

Thanks,
Ira

^ permalink raw reply

* Xilinx LL-Temac driver with two network interfaces
From: Mirek23 @ 2010-01-05  9:45 UTC (permalink / raw)
  To: linuxppc-dev


Dear All,

I am successfully using LL-Temac driver on Virtex-4 cheap under linux 2.6.*.
All works fine with one Network interface. Does anybody know how to
configure the systems to deal with two network interfaces?

I will be grateful for any hint.

M.
-- 
View this message in context: http://old.nabble.com/Xilinx-LL-Temac-driver-with-two-network-interfaces-tp27025912p27025912.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.

^ permalink raw reply

* [PATCH] powerpc: fix pseries/dlpar compile warning without CONFIG_PROC_DEVICETREE
From: FUJITA Tomonori @ 2010-01-05 10:49 UTC (permalink / raw)
  To: linuxppc-dev

cc1: warnings being treated as errors
arch/powerpc/platforms/pseries/dlpar.c: In function 'dlpar_attach_node':
arch/powerpc/platforms/pseries/dlpar.c:239: error: unused variable 'ent'
arch/powerpc/platforms/pseries/dlpar.c: In function 'dlpar_detach_node':
arch/powerpc/platforms/pseries/dlpar.c:271: error: unused variable 'prop'
arch/powerpc/platforms/pseries/dlpar.c:270: error: unused variable 'parent'
make[3]: *** [arch/powerpc/platforms/pseries/dlpar.o] Error 1

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/powerpc/platforms/pseries/dlpar.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 67b7a10..3bd8df8 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -236,7 +236,9 @@ static struct device_node *derive_parent(const char *path)
 
 int dlpar_attach_node(struct device_node *dn)
 {
+#ifdef CONFIG_PROC_DEVICETREE
 	struct proc_dir_entry *ent;
+#endif
 	int rc;
 
 	of_node_set_flag(dn, OF_DYNAMIC);
@@ -267,10 +269,10 @@ int dlpar_attach_node(struct device_node *dn)
 
 int dlpar_detach_node(struct device_node *dn)
 {
+#ifdef CONFIG_PROC_DEVICETREE
 	struct device_node *parent = dn->parent;
 	struct property *prop = dn->properties;
 
-#ifdef CONFIG_PROC_DEVICETREE
 	while (prop) {
 		remove_proc_entry(prop->name, dn->pde);
 		prop = prop->next;
-- 
1.5.6.5

^ permalink raw reply related

* Re: powerpc: allmod config build failure
From: Alexander Graf @ 2010-01-05 13:03 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: Marcelo Tosatti, ppc-dev, Avi Kivity, Hollis Blanchard
In-Reply-To: <20100105143238.ed113985.sfr@canb.auug.org.au>


On 05.01.2010, at 04:32, Stephen Rothwell wrote:

> Hi all,
>=20
> The powerpc allmodconfig build of Linus' tree has been failing like =
this
> since Dec 13 (and linux-next since Nov 7):
>=20
> arch/powerpc/kvm/built-in.o:(.toc1+0x18): undefined reference to =
`kvm_debugfs_dir'
>=20
> $ grep KVM .config
> CONFIG_KVM=3Dy
> CONFIG_KVM_BOOK3S_64_HANDLER=3Dy
> CONFIG_KVM_BOOK3S_64=3Dm
> CONFIG_KVM_EXIT_TIMING=3Dy
>=20
> I assume that this failure was introduced by commit
> c4f9c779f1019c7d5c61961a7c0aaaf1420d8f90 ("Include Book3s_64 target in
> buildsystem").

Yes, and the fix is in Avi's tree already =
(1231730604360801d4e648c37c4b98f30850e001).
I'm not sure how the syncing of those two happens - let's ask him.

Alex=

^ permalink raw reply

* Re: powerpc: allmod config build failure
From: Avi Kivity @ 2010-01-05 13:10 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Stephen Rothwell, Marcelo Tosatti, ppc-dev, Hollis Blanchard
In-Reply-To: <04B8CF10-6D9F-477F-8500-043EA0DEBD78@suse.de>

On 01/05/2010 03:03 PM, Alexander Graf wrote:
> On 05.01.2010, at 04:32, Stephen Rothwell wrote:
>
>    
>> Hi all,
>>
>> The powerpc allmodconfig build of Linus' tree has been failing like this
>> since Dec 13 (and linux-next since Nov 7):
>>
>> arch/powerpc/kvm/built-in.o:(.toc1+0x18): undefined reference to `kvm_debugfs_dir'
>>
>> $ grep KVM .config
>> CONFIG_KVM=y
>> CONFIG_KVM_BOOK3S_64_HANDLER=y
>> CONFIG_KVM_BOOK3S_64=m
>> CONFIG_KVM_EXIT_TIMING=y
>>
>> I assume that this failure was introduced by commit
>> c4f9c779f1019c7d5c61961a7c0aaaf1420d8f90 ("Include Book3s_64 target in
>> buildsystem").
>>      
> Yes, and the fix is in Avi's tree already (1231730604360801d4e648c37c4b98f30850e001).
> I'm not sure how the syncing of those two happens - let's ask him.
>    

Marcelo and myself collect such patches in a branch (kvm.git 
kvm-updates/2.6.33) and push them to Linus.  This week it's Marcelo's turn.

When sending a patch that fixes upstream (or -stable) breakage, please 
not it explicitly, and we'll queue it when applying.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* [PATCH net-next v2 0/3] can: mscan-mpc5xxx: add support for the Freescale MPC512x
From: Wolfgang Grandegger @ 2010-01-05 19:19 UTC (permalink / raw)
  To: Netdev; +Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev

This patch series adds support for the MPC512x from Freescale to
the mpc5xxx_can MSCAN driver.

Changes since v1:

- Various coding style issues, printk formats, variable names and
  error messagaes and typos fixes or improved

- MPC5xxx specific data are now passed to mpc5xxx_can_probe() via
  of_device_id-

Wolfgang Grandegger (3):
  can: mscan: fix improper return if dlc < 8 in start_xmit function
  can: mscan-mpc5xxx: add support for the MPC512x processor
  powerpc/mpc5xxx: add OF platform binding doc for FSL MSCAN devices

 Documentation/powerpc/dts-bindings/fsl/can.txt     |   53 ++++
 Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |    9 +-
 drivers/net/can/mscan/Kconfig                      |    7 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  258 ++++++++++++++++----
 drivers/net/can/mscan/mscan.c                      |   51 +++-
 drivers/net/can/mscan/mscan.h                      |   83 ++++---
 6 files changed, 357 insertions(+), 104 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/fsl/can.txt

^ permalink raw reply

* [PATCH net-next v2 1/3] can: mscan: fix improper return if dlc < 8 in start_xmit function
From: Wolfgang Grandegger @ 2010-01-05 19:19 UTC (permalink / raw)
  To: Netdev
  Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <1262719169-2350-1-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

The start_xmit function of the MSCAN Driver did return improperly if
the CAN dlc check failed (skb not freed and invalid return code). This
patch adds a proper check of the frame lenght and data size and returns
now correctly. Furthermore, a typo has been fixed.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/net/can/mscan/mscan.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index bb06dfb..7df950e 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2005-2006 Andrey Volkov <avolkov@varma-el.com>,
  *                         Varma Electronics Oy
  * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
- * Copytight (C) 2008-2009 Pengutronix <kernel@pengutronix.de>
+ * Copyright (C) 2008-2009 Pengutronix <kernel@pengutronix.de>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the version 2 of the GNU General Public License
@@ -177,8 +177,13 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int i, rtr, buf_id;
 	u32 can_id;
 
-	if (frame->can_dlc > 8)
-		return -EINVAL;
+	if (skb->len != sizeof(*frame) || frame->can_dlc > 8) {
+		dev_err(dev->dev.parent,
+			"Dropping non-conform packet: len %u, can_dlc %u\n",
+			skb->len, frame->can_dlc);
+		kfree_skb(skb);
+		return  NETDEV_TX_OK;
+	}
 
 	out_8(&regs->cantier, 0);
 
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH net-next v2 2/3] can: mscan-mpc5xxx: add support for the MPC512x processor
From: Wolfgang Grandegger @ 2010-01-05 19:19 UTC (permalink / raw)
  To: Netdev
  Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <1262719169-2350-2-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

The main differences compared to the MSCAN on the MPC5200 are:

- More flexibility in choosing the CAN source clock and frequency:

  Three different clock sources can be selected: "ip", "ref" or "sys".
  For the latter two, a clock divider can be defined as well. If the
  clock source is not specified by the device tree, we first try to
  find an optimal CAN source clock based on the system clock. If that
  is not possible, the reference clock will be used.

- The behavior of bus-off recovery is configurable:

  To comply with the usual handling of Socket-CAN bus-off recovery,
  "recovery on request" is selected (instead of automatic recovery).

Note that only MPC5121 Rev. 2 and later is supported.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
---
 drivers/net/can/mscan/Kconfig       |    7 +-
 drivers/net/can/mscan/mpc5xxx_can.c |  258 +++++++++++++++++++++++++++++------
 drivers/net/can/mscan/mscan.c       |   40 +++++-
 drivers/net/can/mscan/mscan.h       |   83 ++++++------
 4 files changed, 295 insertions(+), 93 deletions(-)

diff --git a/drivers/net/can/mscan/Kconfig b/drivers/net/can/mscan/Kconfig
index cd0f2d6..27d1d39 100644
--- a/drivers/net/can/mscan/Kconfig
+++ b/drivers/net/can/mscan/Kconfig
@@ -11,12 +11,13 @@ if CAN_MSCAN
 
 config CAN_MPC5XXX
 	tristate "Freescale MPC5xxx onboard CAN controller"
-	depends on PPC_MPC52xx
+	depends on (PPC_MPC52xx || PPC_MPC512x)
 	---help---
 	  If you say yes here you get support for Freescale's MPC5xxx
-	  onboard CAN controller.
+	  onboard CAN controller. Currently, the MPC5200, MPC5200B and
+	  MPC5121 (Rev. 2 and later) are supported.
 
-	  This driver can also be built as a module.  If so, the module
+	  This driver can also be built as a module. If so, the module
 	  will be called mscan-mpc5xxx.ko.
 
 endif
diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index 1de6f63..022e8be 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -29,6 +29,7 @@
 #include <linux/can/dev.h>
 #include <linux/of_platform.h>
 #include <sysdev/fsl_soc.h>
+#include <linux/clk.h>
 #include <linux/io.h>
 #include <asm/mpc52xx.h>
 
@@ -36,22 +37,21 @@
 
 #define DRV_NAME "mpc5xxx_can"
 
-static struct of_device_id mpc52xx_cdm_ids[] __devinitdata = {
+struct mpc5xxx_can_data {
+	unsigned int type;
+	u32 (*get_clock)(struct of_device *ofdev, const char *clock_name,
+			 int *mscan_clksrc);
+};
+
+#ifdef CONFIG_PPC_MPC5200
+static struct of_device_id __devinitdata mpc52xx_cdm_ids[] = {
 	{ .compatible = "fsl,mpc5200-cdm", },
 	{}
 };
 
-/*
- * Get frequency of the MSCAN clock source
- *
- * Either the oscillator clock (SYS_XTAL_IN) or the IP bus clock (IP_CLK)
- * can be selected. According to the MPC5200 user's manual, the oscillator
- * clock is the better choice as it has less jitter but due to a hardware
- * bug, it can not be selected for the old MPC5200 Rev. A chips.
- */
-
-static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
-						      int clock_src)
+static u32 __devinit mpc52xx_can_get_clock(struct of_device *ofdev,
+					   const char *clock_name,
+					   int *mscan_clksrc)
 {
 	unsigned int pvr;
 	struct mpc52xx_cdm  __iomem *cdm;
@@ -61,11 +61,24 @@ static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
 
 	pvr = mfspr(SPRN_PVR);
 
-	freq = mpc5xxx_get_bus_frequency(of->node);
+	/*
+	 * Either the oscillator clock (SYS_XTAL_IN) or the IP bus clock
+	 * (IP_CLK) can be selected as MSCAN clock source. According to
+	 * the MPC5200 user's manual, the oscillator clock is the better
+	 * choice as it has less jitter. For this reason, it is selected
+	 * by default. Unfortunately, it can not be selected for the old
+	 * MPC5200 Rev. A chips due to a hardware bug (check errata).
+	 */
+	if (clock_name && strcmp(clock_name, "ip") == 0)
+		*mscan_clksrc = MSCAN_CLKSRC_BUS;
+	else
+		*mscan_clksrc = MSCAN_CLKSRC_XTAL;
+
+	freq = mpc5xxx_get_bus_frequency(ofdev->node);
 	if (!freq)
 		return 0;
 
-	if (clock_src == MSCAN_CLKSRC_BUS || pvr == 0x80822011)
+	if (*mscan_clksrc == MSCAN_CLKSRC_BUS || pvr == 0x80822011)
 		return freq;
 
 	/* Determine SYS_XTAL_IN frequency from the clock domain settings */
@@ -75,7 +88,6 @@ static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
 		return 0;
 	}
 	cdm = of_iomap(np_cdm, 0);
-	of_node_put(np_cdm);
 
 	if (in_8(&cdm->ipb_clk_sel) & 0x1)
 		freq *= 2;
@@ -84,26 +96,184 @@ static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
 	freq *= (val & (1 << 5)) ? 8 : 4;
 	freq /= (val & (1 << 6)) ? 12 : 16;
 
+	of_node_put(np_cdm);
 	iounmap(cdm);
 
 	return freq;
 }
+#else /* !CONFIG_PPC_MPC5200 */
+static u32 __devinit mpc52xx_can_get_clock(struct of_device *ofdev,
+					   const char *clock_name,
+					   int *mscan_clksrc)
+{
+	return 0;
+}
+#endif /* CONFIG_PPC_MPC5200 */
+
+#ifdef CONFIG_PPC_MPC512x
+struct mpc512x_clockctl {
+	u32 spmr;		/* System PLL Mode Reg */
+	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
+	u32 scfr1;		/* System Clk Freq Reg 1 */
+	u32 scfr2;		/* System Clk Freq Reg 2 */
+	u32 reserved;
+	u32 bcr;		/* Bread Crumb Reg */
+	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
+	u32 spccr;		/* SPDIF Clk Ctrl Reg */
+	u32 cccr;		/* CFM Clk Ctrl Reg */
+	u32 dccr;		/* DIU Clk Cnfg Reg */
+	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
+};
+
+static struct of_device_id __devinitdata mpc512x_clock_ids[] = {
+	{ .compatible = "fsl,mpc5121-clock", },
+	{}
+};
+
+static u32 __devinit mpc512x_can_get_clock(struct of_device *ofdev,
+					   const char *clock_name,
+					   int *mscan_clksrc)
+{
+	struct mpc512x_clockctl __iomem *clockctl;
+	struct device_node *np_clock;
+	struct clk *sys_clk, *ref_clk;
+	int plen, clockidx, clocksrc = -1;
+	u32 sys_freq, val, clockdiv = 1, freq = 0;
+	const u32 *pval;
+
+	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
+	if (!np_clock) {
+		dev_err(&ofdev->dev, "couldn't find clock node\n");
+		return -ENODEV;
+	}
+	clockctl = of_iomap(np_clock, 0);
+	if (!clockctl) {
+		dev_err(&ofdev->dev, "couldn't map clock registers\n");
+		return 0;
+	}
+
+	/* Determine the MSCAN device index from the physical address */
+	pval = of_get_property(ofdev->node, "reg", &plen);
+	BUG_ON(!pval || plen < sizeof(*pval));
+	clockidx = (*pval & 0x80) ? 1 : 0;
+	if (*pval & 0x2000)
+		clockidx += 2;
+
+	/*
+	 * Clock source and divider selection: 3 different clock sources
+	 * can be selected: "ip", "ref" or "sys". For the latter two, a
+	 * clock divider can be defined as well. If the clock source is
+	 * not specified by the device tree, we first try to find an
+	 * optimal CAN source clock based on the system clock. If that
+	 * is not posslible, the reference clock will be used.
+	 */
+	if (clock_name && !strcmp(clock_name, "ip")) {
+		*mscan_clksrc = MSCAN_CLKSRC_IPS;
+		freq = mpc5xxx_get_bus_frequency(ofdev->node);
+	} else {
+		*mscan_clksrc = MSCAN_CLKSRC_BUS;
+
+		pval = of_get_property(ofdev->node,
+				       "fsl,mscan-clock-divider", &plen);
+		if (pval && plen == sizeof(*pval))
+			clockdiv = *pval;
+		if (!clockdiv)
+			clockdiv = 1;
+
+		if (!clock_name || !strcmp(clock_name, "sys")) {
+			sys_clk = clk_get(&ofdev->dev, "sys_clk");
+			if (!sys_clk) {
+				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
+				goto exit_unmap;
+			}
+			/* Get and round up/down sys clock rate */
+			sys_freq = 1000000 *
+				((clk_get_rate(sys_clk) + 499999) / 1000000);
+
+			if (!clock_name) {
+				/* A multiple of 16 MHz would be optimal */
+				if ((sys_freq % 16000000) == 0) {
+					clocksrc = 0;
+					clockdiv = sys_freq / 16000000;
+					freq = sys_freq / clockdiv;
+				}
+			} else {
+				clocksrc = 0;
+				freq = sys_freq / clockdiv;
+			}
+		}
+
+		if (clocksrc < 0) {
+			ref_clk = clk_get(&ofdev->dev, "ref_clk");
+			if (!ref_clk) {
+				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
+				goto exit_unmap;
+			}
+			clocksrc = 1;
+			freq = clk_get_rate(ref_clk) / clockdiv;
+		}
+	}
+
+	/* Disable clock */
+	out_be32(&clockctl->mccr[clockidx], 0x0);
+	if (clocksrc >= 0) {
+		/* Set source and divider */
+		val = (clocksrc << 14) | ((clockdiv - 1) << 17);
+		out_be32(&clockctl->mccr[clockidx], val);
+		/* Enable clock */
+		out_be32(&clockctl->mccr[clockidx], val | 0x10000);
+	}
+
+	/* Enable MSCAN clock domain */
+	val = in_be32(&clockctl->sccr[1]);
+	if (!(val & (1 << 25)))
+		out_be32(&clockctl->sccr[1], val | (1 << 25));
+
+	dev_dbg(&ofdev->dev, "using '%s' with frequency divider %d\n",
+		*mscan_clksrc == MSCAN_CLKSRC_IPS ? "ips_clk" :
+		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
+
+exit_unmap:
+	of_node_put(np_clock);
+	iounmap(clockctl);
+
+	return freq;
+}
+#else /* !CONFIG_PPC_MPC512x */
+static u32 __devinit mpc512x_can_get_clock(struct of_device *ofdev,
+					   const char *clock_name,
+					   int *mscan_clksrc)
+{
+	return 0;
+}
+#endif /* CONFIG_PPC_MPC512x */
+
+static struct mpc5xxx_can_data __devinitdata mpc5200_can_data = {
+	.type = MSCAN_TYPE_MPC5200,
+	.get_clock = mpc52xx_can_get_clock,
+};
+
+static struct mpc5xxx_can_data __devinitdata mpc5121_can_data = {
+	.type = MSCAN_TYPE_MPC5121,
+	.get_clock = mpc512x_can_get_clock,
+};
 
 static int __devinit mpc5xxx_can_probe(struct of_device *ofdev,
 				       const struct of_device_id *id)
 {
+	struct mpc5xxx_can_data *data = (struct mpc5xxx_can_data *)id->data;
 	struct device_node *np = ofdev->node;
 	struct net_device *dev;
 	struct mscan_priv *priv;
 	void __iomem *base;
-	const char *clk_src;
-	int err, irq, clock_src;
+	const char *clock_name = NULL;
+	int irq, mscan_clksrc = 0;
+	int err = -ENOMEM;
 
-	base = of_iomap(ofdev->node, 0);
+	base = of_iomap(np, 0);
 	if (!base) {
 		dev_err(&ofdev->dev, "couldn't ioremap\n");
-		err = -ENOMEM;
-		goto exit_release_mem;
+		return err;
 	}
 
 	irq = irq_of_parse_and_map(np, 0);
@@ -114,37 +284,27 @@ static int __devinit mpc5xxx_can_probe(struct of_device *ofdev,
 	}
 
 	dev = alloc_mscandev();
-	if (!dev) {
-		err = -ENOMEM;
+	if (!dev)
 		goto exit_dispose_irq;
-	}
 
 	priv = netdev_priv(dev);
 	priv->reg_base = base;
 	dev->irq = irq;
 
-	/*
-	 * Either the oscillator clock (SYS_XTAL_IN) or the IP bus clock
-	 * (IP_CLK) can be selected as MSCAN clock source. According to
-	 * the MPC5200 user's manual, the oscillator clock is the better
-	 * choice as it has less jitter. For this reason, it is selected
-	 * by default.
-	 */
-	clk_src = of_get_property(np, "fsl,mscan-clock-source", NULL);
-	if (clk_src && strcmp(clk_src, "ip") == 0)
-		clock_src = MSCAN_CLKSRC_BUS;
-	else
-		clock_src = MSCAN_CLKSRC_XTAL;
-	priv->can.clock.freq = mpc52xx_can_clock_freq(ofdev, clock_src);
+	clock_name = of_get_property(np, "fsl,mscan-clock-source", NULL);
+
+	BUG_ON(!data);
+	priv->type = data->type;
+	priv->can.clock.freq = data->get_clock(ofdev, clock_name,
+					       &mscan_clksrc);
 	if (!priv->can.clock.freq) {
-		dev_err(&ofdev->dev, "couldn't get MSCAN clock frequency\n");
-		err = -ENODEV;
+		dev_err(&ofdev->dev, "couldn't get MSCAN clock properties\n");
 		goto exit_free_mscan;
 	}
 
 	SET_NETDEV_DEV(dev, &ofdev->dev);
 
-	err = register_mscandev(dev, clock_src);
+	err = register_mscandev(dev, mscan_clksrc);
 	if (err) {
 		dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
 			DRV_NAME, err);
@@ -164,7 +324,7 @@ exit_dispose_irq:
 	irq_dispose_mapping(irq);
 exit_unmap_mem:
 	iounmap(base);
-exit_release_mem:
+
 	return err;
 }
 
@@ -225,8 +385,22 @@ static int mpc5xxx_can_resume(struct of_device *ofdev)
 }
 #endif
 
+#if 0
+static struct mpc5xxx_can_data __devinitdata mpc5200_can_data = {
+	.type = MSCAN_TYPE_MPC5200,
+	.get_clock = mpc52xx_can_get_clock,
+};
+
+static struct mpc5xxx_can_data __devinitdata mpc5121_can_data = {
+	.type = MSCAN_TYPE_MPC5121,
+	.get_clock = mpc512x_can_get_clock,
+};
+#endif
+
 static struct of_device_id __devinitdata mpc5xxx_can_table[] = {
-	{.compatible = "fsl,mpc5200-mscan"},
+	{ .compatible = "fsl,mpc5200-mscan", .data = &mpc5200_can_data },
+	/* Note that only MPC5121 Rev. 2 (and later) is supported */
+	{ .compatible = "fsl,mpc5121-mscan", .data = &mpc5121_can_data },
 	{},
 };
 
@@ -255,5 +429,5 @@ static void __exit mpc5xxx_can_exit(void)
 module_exit(mpc5xxx_can_exit);
 
 MODULE_AUTHOR("Wolfgang Grandegger <wg@grandegger.com>");
-MODULE_DESCRIPTION("Freescale MPC5200 CAN driver");
+MODULE_DESCRIPTION("Freescale MPC5xxx CAN driver");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index 7df950e..d2c353f 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -169,6 +169,26 @@ static int mscan_start(struct net_device *dev)
 	return 0;
 }
 
+static int mscan_restart(struct net_device *dev)
+{
+	struct mscan_priv *priv = netdev_priv(dev);
+
+	if (priv->type == MSCAN_TYPE_MPC5121) {
+		struct mscan_regs *regs = (struct mscan_regs *)priv->reg_base;
+
+		priv->can.state = CAN_STATE_ERROR_ACTIVE;
+		WARN(!(in_8(&regs->canmisc) & MSCAN_BOHOLD),
+		     "bus-off state expected");
+		out_8(&regs->canmisc, MSCAN_BOHOLD);
+	} else {
+		if (priv->can.state <= CAN_STATE_BUS_OFF)
+			mscan_set_mode(dev, MSCAN_INIT_MODE);
+		return mscan_start(dev);
+	}
+
+	return 0;
+}
+
 static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct can_frame *frame = (struct can_frame *)skb->data;
@@ -363,9 +383,12 @@ static void mscan_get_err_frame(struct net_device *dev, struct can_frame *frame,
 			 * automatically. To avoid that we stop the chip doing
 			 * a light-weight stop (we are in irq-context).
 			 */
-			out_8(&regs->cantier, 0);
-			out_8(&regs->canrier, 0);
-			setbits8(&regs->canctl0, MSCAN_SLPRQ | MSCAN_INITRQ);
+			if (priv->type != MSCAN_TYPE_MPC5121) {
+				out_8(&regs->cantier, 0);
+				out_8(&regs->canrier, 0);
+				setbits8(&regs->canctl0,
+					 MSCAN_SLPRQ | MSCAN_INITRQ);
+			}
 			can_bus_off(dev);
 			break;
 		default:
@@ -495,9 +518,7 @@ static int mscan_do_set_mode(struct net_device *dev, enum can_mode mode)
 
 	switch (mode) {
 	case CAN_MODE_START:
-		if (priv->can.state <= CAN_STATE_BUS_OFF)
-			mscan_set_mode(dev, MSCAN_INIT_MODE);
-		ret = mscan_start(dev);
+		ret = mscan_restart(dev);
 		if (ret)
 			break;
 		if (netif_queue_stopped(dev))
@@ -596,18 +617,21 @@ static const struct net_device_ops mscan_netdev_ops = {
        .ndo_start_xmit         = mscan_start_xmit,
 };
 
-int register_mscandev(struct net_device *dev, int clock_src)
+int register_mscandev(struct net_device *dev, int mscan_clksrc)
 {
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs *regs = (struct mscan_regs *)priv->reg_base;
 	u8 ctl1;
 
 	ctl1 = in_8(&regs->canctl1);
-	if (clock_src)
+	if (mscan_clksrc)
 		ctl1 |= MSCAN_CLKSRC;
 	else
 		ctl1 &= ~MSCAN_CLKSRC;
 
+	if (priv->type == MSCAN_TYPE_MPC5121)
+		ctl1 |= MSCAN_BORM; /* bus-off recovery upon request */
+
 	ctl1 |= MSCAN_CANE;
 	out_8(&regs->canctl1, ctl1);
 	udelay(100);
diff --git a/drivers/net/can/mscan/mscan.h b/drivers/net/can/mscan/mscan.h
index 00fc4aa..897e5ac 100644
--- a/drivers/net/can/mscan/mscan.h
+++ b/drivers/net/can/mscan/mscan.h
@@ -38,18 +38,20 @@
 #define MSCAN_CLKSRC		0x40
 #define MSCAN_LOOPB		0x20
 #define MSCAN_LISTEN		0x10
+#define MSCAN_BORM		0x08
 #define MSCAN_WUPM		0x04
 #define MSCAN_SLPAK		0x02
 #define MSCAN_INITAK		0x01
 
-/* Use the MPC5200 MSCAN variant? */
+/* Use the MPC5XXX MSCAN variant? */
 #ifdef CONFIG_PPC
-#define MSCAN_FOR_MPC5200
+#define MSCAN_FOR_MPC5XXX
 #endif
 
-#ifdef MSCAN_FOR_MPC5200
+#ifdef MSCAN_FOR_MPC5XXX
 #define MSCAN_CLKSRC_BUS	0
 #define MSCAN_CLKSRC_XTAL	MSCAN_CLKSRC
+#define MSCAN_CLKSRC_IPS	MSCAN_CLKSRC
 #else
 #define MSCAN_CLKSRC_BUS	MSCAN_CLKSRC
 #define MSCAN_CLKSRC_XTAL	0
@@ -136,7 +138,7 @@
 #define MSCAN_EFF_RTR_SHIFT	0
 #define MSCAN_EFF_FLAGS		0x18	/* IDE + SRR */
 
-#ifdef MSCAN_FOR_MPC5200
+#ifdef MSCAN_FOR_MPC5XXX
 #define _MSCAN_RESERVED_(n, num) u8 _res##n[num]
 #define _MSCAN_RESERVED_DSR_SIZE	2
 #else
@@ -165,67 +167,66 @@ struct mscan_regs {
 	u8 cantbsel;				/* + 0x14     0x0a */
 	u8 canidac;				/* + 0x15     0x0b */
 	u8 reserved;				/* + 0x16     0x0c */
-	_MSCAN_RESERVED_(6, 5);			/* + 0x17          */
-#ifndef MSCAN_FOR_MPC5200
-	u8 canmisc;				/*            0x0d */
-#endif
+	_MSCAN_RESERVED_(6, 2);			/* + 0x17          */
+	u8 canmisc;				/* + 0x19     0x0d */
+	_MSCAN_RESERVED_(7, 2);			/* + 0x1a          */
 	u8 canrxerr;				/* + 0x1c     0x0e */
 	u8 cantxerr;				/* + 0x1d     0x0f */
-	_MSCAN_RESERVED_(7, 2);			/* + 0x1e          */
+	_MSCAN_RESERVED_(8, 2);			/* + 0x1e          */
 	u16 canidar1_0;				/* + 0x20     0x10 */
-	_MSCAN_RESERVED_(8, 2);			/* + 0x22          */
+	_MSCAN_RESERVED_(9, 2);			/* + 0x22          */
 	u16 canidar3_2;				/* + 0x24     0x12 */
-	_MSCAN_RESERVED_(9, 2);			/* + 0x26          */
+	_MSCAN_RESERVED_(10, 2);		/* + 0x26          */
 	u16 canidmr1_0;				/* + 0x28     0x14 */
-	_MSCAN_RESERVED_(10, 2);		/* + 0x2a          */
+	_MSCAN_RESERVED_(11, 2);		/* + 0x2a          */
 	u16 canidmr3_2;				/* + 0x2c     0x16 */
-	_MSCAN_RESERVED_(11, 2);		/* + 0x2e          */
+	_MSCAN_RESERVED_(12, 2);		/* + 0x2e          */
 	u16 canidar5_4;				/* + 0x30     0x18 */
-	_MSCAN_RESERVED_(12, 2);		/* + 0x32          */
+	_MSCAN_RESERVED_(13, 2);		/* + 0x32          */
 	u16 canidar7_6;				/* + 0x34     0x1a */
-	_MSCAN_RESERVED_(13, 2);		/* + 0x36          */
+	_MSCAN_RESERVED_(14, 2);		/* + 0x36          */
 	u16 canidmr5_4;				/* + 0x38     0x1c */
-	_MSCAN_RESERVED_(14, 2);		/* + 0x3a          */
+	_MSCAN_RESERVED_(15, 2);		/* + 0x3a          */
 	u16 canidmr7_6;				/* + 0x3c     0x1e */
-	_MSCAN_RESERVED_(15, 2);		/* + 0x3e          */
+	_MSCAN_RESERVED_(16, 2);		/* + 0x3e          */
 	struct {
 		u16 idr1_0;			/* + 0x40     0x20 */
-		 _MSCAN_RESERVED_(16, 2);	/* + 0x42          */
+		_MSCAN_RESERVED_(17, 2);	/* + 0x42          */
 		u16 idr3_2;			/* + 0x44     0x22 */
-		 _MSCAN_RESERVED_(17, 2);	/* + 0x46          */
+		_MSCAN_RESERVED_(18, 2);	/* + 0x46          */
 		u16 dsr1_0;			/* + 0x48     0x24 */
-		 _MSCAN_RESERVED_(18, 2);	/* + 0x4a          */
+		_MSCAN_RESERVED_(19, 2);	/* + 0x4a          */
 		u16 dsr3_2;			/* + 0x4c     0x26 */
-		 _MSCAN_RESERVED_(19, 2);	/* + 0x4e          */
+		_MSCAN_RESERVED_(20, 2);	/* + 0x4e          */
 		u16 dsr5_4;			/* + 0x50     0x28 */
-		 _MSCAN_RESERVED_(20, 2);	/* + 0x52          */
+		_MSCAN_RESERVED_(21, 2);	/* + 0x52          */
 		u16 dsr7_6;			/* + 0x54     0x2a */
-		 _MSCAN_RESERVED_(21, 2);	/* + 0x56          */
+		_MSCAN_RESERVED_(22, 2);	/* + 0x56          */
 		u8 dlr;				/* + 0x58     0x2c */
-		 u8:8;				/* + 0x59     0x2d */
-		 _MSCAN_RESERVED_(22, 2);	/* + 0x5a          */
+		u8 reserved;			/* + 0x59     0x2d */
+		_MSCAN_RESERVED_(23, 2);	/* + 0x5a          */
 		u16 time;			/* + 0x5c     0x2e */
 	} rx;
-	 _MSCAN_RESERVED_(23, 2);		/* + 0x5e          */
+	_MSCAN_RESERVED_(24, 2);		/* + 0x5e          */
 	struct {
 		u16 idr1_0;			/* + 0x60     0x30 */
-		 _MSCAN_RESERVED_(24, 2);	/* + 0x62          */
+		_MSCAN_RESERVED_(25, 2);	/* + 0x62          */
 		u16 idr3_2;			/* + 0x64     0x32 */
-		 _MSCAN_RESERVED_(25, 2);	/* + 0x66          */
+		_MSCAN_RESERVED_(26, 2);	/* + 0x66          */
 		u16 dsr1_0;			/* + 0x68     0x34 */
-		 _MSCAN_RESERVED_(26, 2);	/* + 0x6a          */
+		_MSCAN_RESERVED_(27, 2);	/* + 0x6a          */
 		u16 dsr3_2;			/* + 0x6c     0x36 */
-		 _MSCAN_RESERVED_(27, 2);	/* + 0x6e          */
+		_MSCAN_RESERVED_(28, 2);	/* + 0x6e          */
 		u16 dsr5_4;			/* + 0x70     0x38 */
-		 _MSCAN_RESERVED_(28, 2);	/* + 0x72          */
+		_MSCAN_RESERVED_(29, 2);	/* + 0x72          */
 		u16 dsr7_6;			/* + 0x74     0x3a */
-		 _MSCAN_RESERVED_(29, 2);	/* + 0x76          */
+		_MSCAN_RESERVED_(30, 2);	/* + 0x76          */
 		u8 dlr;				/* + 0x78     0x3c */
 		u8 tbpr;			/* + 0x79     0x3d */
-		 _MSCAN_RESERVED_(30, 2);	/* + 0x7a          */
+		_MSCAN_RESERVED_(31, 2);	/* + 0x7a          */
 		u16 time;			/* + 0x7c     0x3e */
 	} tx;
-	 _MSCAN_RESERVED_(31, 2);		/* + 0x7e          */
+	_MSCAN_RESERVED_(32, 2);		/* + 0x7e          */
 } __attribute__ ((packed));
 
 #undef _MSCAN_RESERVED_
@@ -238,6 +239,12 @@ struct mscan_regs {
 #define MSCAN_SET_MODE_RETRIES	255
 #define MSCAN_ECHO_SKB_MAX	3
 
+/* MSCAN type variants */
+enum {
+	MSCAN_TYPE_MPC5200,
+	MSCAN_TYPE_MPC5121
+};
+
 #define BTR0_BRP_MASK		0x3f
 #define BTR0_SJW_SHIFT		6
 #define BTR0_SJW_MASK		(0x3 << BTR0_SJW_SHIFT)
@@ -270,6 +277,7 @@ struct tx_queue_entry {
 
 struct mscan_priv {
 	struct can_priv can;	/* must be the first member */
+	unsigned int type; 	/* MSCAN type variants */
 	long open_time;
 	unsigned long flags;
 	void __iomem *reg_base;	/* ioremap'ed address to registers */
@@ -285,12 +293,7 @@ struct mscan_priv {
 };
 
 extern struct net_device *alloc_mscandev(void);
-/*
- * clock_src:
- *	1 = The MSCAN clock source is the onchip Bus Clock.
- *	0 = The MSCAN clock source is the chip Oscillator Clock.
- */
-extern int register_mscandev(struct net_device *dev, int clock_src);
+extern int register_mscandev(struct net_device *dev, int mscan_clksrc);
 extern void unregister_mscandev(struct net_device *dev);
 
 #endif /* __MSCAN_H__ */
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH net-next v2 3/3] powerpc/mpc5xxx: add OF platform binding doc for FSL MSCAN devices
From: Wolfgang Grandegger @ 2010-01-05 19:19 UTC (permalink / raw)
  To: Netdev
  Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <1262719169-2350-3-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

This patch adds documentation for the MSCAN OF device bindings for
the MPC512x and moves the one for the MPC5200 to the new common file
"Documentation/powerpc/dts-bindings/fsl/can.txt".

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 Documentation/powerpc/dts-bindings/fsl/can.txt     |   53 ++++++++++++++++++++
 Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |    9 +---
 2 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/fsl/can.txt

diff --git a/Documentation/powerpc/dts-bindings/fsl/can.txt b/Documentation/powerpc/dts-bindings/fsl/can.txt
new file mode 100644
index 0000000..2fa4fcd
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/fsl/can.txt
@@ -0,0 +1,53 @@
+CAN Device Tree Bindings
+------------------------
+
+(c) 2006-2009 Secret Lab Technologies Ltd
+Grant Likely <grant.likely@secretlab.ca>
+
+fsl,mpc5200-mscan nodes
+-----------------------
+In addition to the required compatible-, reg- and interrupt-properties, you can
+also specify which clock source shall be used for the controller:
+
+- fsl,mscan-clock-source : a string describing the clock source. Valid values
+			   are:	"ip" for ip bus clock
+				 "ref" for reference clock (XTAL)
+			   "ref" is default in case this property is not
+			   present.
+
+fsl,mpc5121-mscan nodes
+-----------------------
+In addition to the required compatible-, reg- and interrupt-properties, you can
+also specify which clock source and divider shall be used for the controller:
+
+- fsl,mscan-clock-source : a string describing the clock source. Valid values
+			   are:	"ip" for ip bus clock
+				"ref" for reference clock
+				"sys" for system clock
+			   If this property is not present, an optimal CAN
+			   clock source and frequency based on the system
+			   clock will be selected. If this is not possible,
+			   the reference clock will be used.
+
+- fsl,mscan-clock-divider: for the reference and system clock, an additional
+			   clock divider can be specified. By default, a
+			   value of 1 is used.
+
+Note that the MPC5121 Rev. 1 processor is not supported.
+
+Examples:
+	can@1300 {
+		compatible = "fsl,mpc5121-mscan";
+		interrupts = <12 0x8>;
+		interrupt-parent = <&ipic>;
+		reg = <0x1300 0x80>;
+	};
+
+	can@1380 {
+		compatible = "fsl,mpc5121-mscan";
+		interrupts = <13 0x8>;
+		interrupt-parent = <&ipic>;
+		reg = <0x1380 0x80>;
+		fsl,mscan-clock-source = "ref";
+		fsl,mscan-clock-divider = <3>;
+	};
diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
index cabc780..95accfa 100644
--- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
@@ -180,11 +180,4 @@ External interrupts:
 
 fsl,mpc5200-mscan nodes
 -----------------------
-In addition to the required compatible-, reg- and interrupt-properites, you can
-also specify which clock source shall be used for the controller:
-
-- fsl,mscan-clock-source- a string describing the clock source. Valid values
-			  are:	"ip" for ip bus clock
-				"ref" for reference clock (XTAL)
-			  "ref" is default in case this property is not
-			  present.
+See file can.txt in this directory.
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH net-next v2 0/3] can: mscan-mpc5xxx: add support for the Freescale MPC512x
From: Wolfgang Grandegger @ 2010-01-05 19:20 UTC (permalink / raw)
  To: Netdev; +Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev

This patch series adds support for the MPC512x from Freescale to
the mpc5xxx_can MSCAN driver.

Changes since v1:

- Various coding style issues, printk formats, variable names and
  error messagaes and typos fixes or improved

- MPC5xxx specific data are now passed to mpc5xxx_can_probe() via
  "of_device_id->data".

- The index of the MPC512x CAN controller is now derived directly
  from reg property. This allows use of_iomap() as usual.

- It has been documented that MPC512x Rev.1 CPUs are not supported.

Wolfgang

Wolfgang Grandegger (3):
  can: mscan: fix improper return if dlc < 8 in start_xmit function
  can: mscan-mpc5xxx: add support for the MPC512x processor
  powerpc/mpc5xxx: add OF platform binding doc for FSL MSCAN devices

 Documentation/powerpc/dts-bindings/fsl/can.txt     |   53 ++++
 Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |    9 +-
 drivers/net/can/mscan/Kconfig                      |    7 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  258 ++++++++++++++++----
 drivers/net/can/mscan/mscan.c                      |   51 +++-
 drivers/net/can/mscan/mscan.h                      |   83 ++++---
 6 files changed, 357 insertions(+), 104 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/fsl/can.txt

^ permalink raw reply

* [PATCH net-next v2 1/3] can: mscan: fix improper return if dlc < 8 in start_xmit function
From: Wolfgang Grandegger @ 2010-01-05 19:20 UTC (permalink / raw)
  To: Netdev
  Cc: Socketcan-core, Devicetree-discuss, Linuxppc-dev,
	Wolfgang Grandegger
In-Reply-To: <1262719241-2422-1-git-send-email-wg@grandegger.com>

From: Wolfgang Grandegger <wg@denx.de>

The start_xmit function of the MSCAN Driver did return improperly if
the CAN dlc check failed (skb not freed and invalid return code). This
patch adds a proper check of the frame lenght and data size and returns
now correctly. Furthermore, a typo has been fixed.

Signed-off-by: Wolfgang Grandegger <wg@denx.de>
Acked-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/net/can/mscan/mscan.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
index bb06dfb..7df950e 100644
--- a/drivers/net/can/mscan/mscan.c
+++ b/drivers/net/can/mscan/mscan.c
@@ -4,7 +4,7 @@
  * Copyright (C) 2005-2006 Andrey Volkov <avolkov@varma-el.com>,
  *                         Varma Electronics Oy
  * Copyright (C) 2008-2009 Wolfgang Grandegger <wg@grandegger.com>
- * Copytight (C) 2008-2009 Pengutronix <kernel@pengutronix.de>
+ * Copyright (C) 2008-2009 Pengutronix <kernel@pengutronix.de>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the version 2 of the GNU General Public License
@@ -177,8 +177,13 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int i, rtr, buf_id;
 	u32 can_id;
 
-	if (frame->can_dlc > 8)
-		return -EINVAL;
+	if (skb->len != sizeof(*frame) || frame->can_dlc > 8) {
+		dev_err(dev->dev.parent,
+			"Dropping non-conform packet: len %u, can_dlc %u\n",
+			skb->len, frame->can_dlc);
+		kfree_skb(skb);
+		return  NETDEV_TX_OK;
+	}
 
 	out_8(&regs->cantier, 0);
 
-- 
1.6.2.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox