* [PATCH] fix EMAC driver for proper napi_synchronize API
From: Benjamin Herrenschmidt @ 2007-10-17 23:14 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Roland Dreier, David S. Miller, Jeff Garzik,
linuxppc-dev list
In-Reply-To: <20071017083107.6a1b1bd3@freepuppy.rosehill>
The EMAC driver "fix" was merged by mistake before the dust had settled on
the new napi synchronize interface (and before it got merged). The final
version of that function is spelled without underscores.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Index: linux-work/drivers/net/ibm_newemac/mal.c
===================================================================
--- linux-work.orig/drivers/net/ibm_newemac/mal.c 2007-10-18 08:02:22.000000000 +1000
+++ linux-work/drivers/net/ibm_newemac/mal.c 2007-10-18 08:02:24.000000000 +1000
@@ -322,7 +322,7 @@ void mal_poll_disable(struct mal_instanc
msleep(1);
/* Synchronize with the MAL NAPI poller */
- __napi_synchronize(&mal->napi);
+ napi_synchronize(&mal->napi);
}
void mal_poll_enable(struct mal_instance *mal, struct mal_commac *commac)
^ permalink raw reply
* [PATCH] Move _rtc_time() routines under CONFIG_ADB_CUDA
From: Badari Pulavarty @ 2007-10-17 23:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: lkml
Fix to clean up compile warnings (against 2.6.23-git12)
Thanks,
Badari
to_rtc_time() and from_rtc_time() seems to be used only if CONFIG_ADB_CUDA
defined. Moving them under that ifdef.
arch/powerpc/platforms/powermac/time.c:88: warning: `to_rtc_time' defined but not used
arch/powerpc/platforms/powermac/time.c:95: warning: `from_rtc_time' defined but not used
Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
---
arch/powerpc/platforms/powermac/time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.23/arch/powerpc/platforms/powermac/time.c
===================================================================
--- linux-2.6.23.orig/arch/powerpc/platforms/powermac/time.c 2007-10-09 13:31:38.000000000 -0700
+++ linux-2.6.23/arch/powerpc/platforms/powermac/time.c 2007-10-17 15:59:56.000000000 -0700
@@ -84,6 +84,7 @@ long __init pmac_time_init(void)
return delta;
}
+#ifdef CONFIG_ADB_CUDA
static void to_rtc_time(unsigned long now, struct rtc_time *tm)
{
to_tm(now, tm);
@@ -97,7 +98,6 @@ static unsigned long from_rtc_time(struc
tm->tm_hour, tm->tm_min, tm->tm_sec);
}
-#ifdef CONFIG_ADB_CUDA
static unsigned long cuda_get_time(void)
{
struct adb_request req;
^ permalink raw reply
* Re: [PATCH 2/2] Use of_get_pci_dev_node() in axon_msi.c
From: Linas Vepstas @ 2007-10-17 23:04 UTC (permalink / raw)
To: Michael Ellerman
Cc: sparclinux, Stephen Rothwell, Paul Mackerras, David S. Miller,
linuxppc-dev
In-Reply-To: <dadd2b9540369b9e2c6773b2a60021dadafa58e2.1192605144.git.michael@ellerman.id.au>
On Wed, Oct 17, 2007 at 05:12:27PM +1000, Michael Ellerman wrote:
> +struct device_node *of_get_pci_dev_node(struct pci_dev *pdev)
> +{
> + return of_node_get(pci_device_to_OF_node(pdev));
> +}
[...]
> - dn = of_node_get(pci_device_to_OF_node(dev));
> + dn = of_get_pci_dev_node(dev);
Is this really useful or wise?
As a matter of personal taste, I find stuff like this clutters
and confuses my mind. I go to read new code, and I run across some
routine I haven't heard of before ... e.g. of_get_pci_dev_node(),
so now I have to look it up to see what it does. A few minutes later,
I realize that its just a pair of old freinds (of_node_get and
pci_device_to_OF_node) and so now I have to make mental room for it.
Tommorrow, or 3 days later, I'm again looking at of_get_pci_dev_node()
and I'm thinking "gee what did that thing do again??"
I don't much like this style, and I've been known to submit
patches that remove stuff like this ...
--linas
^ permalink raw reply
* Re: [PATCH] fs_enet: Update for API changes
From: Vitaly Bordug @ 2007-10-17 22:40 UTC (permalink / raw)
To: Scott Wood; +Cc: netdev, Jeff Garzik, linuxppc-dev
In-Reply-To: <20071017174243.GA4462@loki.buserror.net>
Hello Scott,
On Wed, 17 Oct 2007 12:42:43 -0500
Scott Wood wrote:
> This driver was recently broken by several changes for which this
> driver was not (or was improperly) updated:
>
> 1. SET_MODULE_OWNER() was removed.
> 2. netif_napi_add() was only being called when building with
> the old CPM binding.
> 3. The received/budget test was backwards.
> 4. to_net_dev() was wrong -- the device struct embedded in
> the net_device struct is not the same as the of_platform
> device in the private struct.
> 5. napi_disable/napi_enable was being called even when napi
> was not being used.
>
Good cleanup, thanks!
Jeff: this is important fix, and should be applied if possible.
> These changes have been fixed, and napi is now on by default.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Acked-by: Vitaly Bordug <vitb@kernel.crashing.org>
> ---
> drivers/net/fs_enet/fs_enet-main.c | 28 ++++++++++++++++------------
> drivers/net/fs_enet/fs_enet.h | 1 +
> 2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> index 04c6fae..f2a4d39 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -88,7 +88,7 @@ static void skb_align(struct sk_buff *skb, int align)
> static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
> {
> struct fs_enet_private *fep = container_of(napi, struct fs_enet_private, napi);
> - struct net_device *dev = to_net_dev(fep->dev);
> + struct net_device *dev = fep->ndev;
> const struct fs_platform_info *fpi = fep->fpi;
> cbd_t __iomem *bdp;
> struct sk_buff *skb, *skbn, *skbt;
> @@ -217,7 +217,7 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int budget)
>
> fep->cur_rx = bdp;
>
> - if (received >= budget) {
> + if (received < budget) {
> /* done */
> netif_rx_complete(dev, napi);
> (*fep->ops->napi_enable_rx)(dev);
> @@ -807,20 +807,23 @@ static int fs_enet_open(struct net_device *dev)
> int r;
> int err;
>
> - napi_enable(&fep->napi);
> + if (fep->fpi->use_napi)
> + napi_enable(&fep->napi);
>
> /* Install our interrupt handler. */
> r = fs_request_irq(dev, fep->interrupt, "fs_enet-mac", fs_enet_interrupt);
> if (r != 0) {
> printk(KERN_ERR DRV_MODULE_NAME
> ": %s Could not allocate FS_ENET IRQ!", dev->name);
> - napi_disable(&fep->napi);
> + if (fep->fpi->use_napi)
> + napi_disable(&fep->napi);
> return -EINVAL;
> }
>
> err = fs_init_phy(dev);
> - if(err) {
> - napi_disable(&fep->napi);
> + if (err) {
> + if (fep->fpi->use_napi)
> + napi_disable(&fep->napi);
> return err;
> }
> phy_start(fep->phydev);
> @@ -1232,7 +1235,7 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
> fpi->rx_ring = 32;
> fpi->tx_ring = 32;
> fpi->rx_copybreak = 240;
> - fpi->use_napi = 0;
> + fpi->use_napi = 1;
> fpi->napi_weight = 17;
>
> ret = find_phy(ofdev->node, fpi);
> @@ -1249,11 +1252,11 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
> goto out_free_fpi;
> }
>
> - SET_MODULE_OWNER(ndev);
> dev_set_drvdata(&ofdev->dev, ndev);
>
> fep = netdev_priv(ndev);
> fep->dev = &ofdev->dev;
> + fep->ndev = ndev;
> fep->fpi = fpi;
> fep->ops = match->data;
>
> @@ -1288,10 +1291,11 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
> ndev->stop = fs_enet_close;
> ndev->get_stats = fs_enet_get_stats;
> ndev->set_multicast_list = fs_set_multicast_list;
> - if (fpi->use_napi) {
> - ndev->poll = fs_enet_rx_napi;
> - ndev->weight = fpi->napi_weight;
> - }
> +
> + if (fpi->use_napi)
> + netif_napi_add(ndev, &fep->napi, fs_enet_rx_napi,
> + fpi->napi_weight);
> +
> ndev->ethtool_ops = &fs_ethtool_ops;
> ndev->do_ioctl = fs_ioctl;
>
> diff --git a/drivers/net/fs_enet/fs_enet.h b/drivers/net/fs_enet/fs_enet.h
> index baf6477..c675e29 100644
> --- a/drivers/net/fs_enet/fs_enet.h
> +++ b/drivers/net/fs_enet/fs_enet.h
> @@ -75,6 +75,7 @@ struct phy_info {
> struct fs_enet_private {
> struct napi_struct napi;
> struct device *dev; /* pointer back to the device (must be initialized first) */
> + struct net_device *ndev;
> spinlock_t lock; /* during all ops except TX pckt processing */
> spinlock_t tx_lock; /* during fs_start_xmit and fs_tx */
> struct fs_platform_info *fpi;
--
Sincerely, Vitaly
^ permalink raw reply
* Re: [PATCH 1/2] qemu platform, v2
From: Rob Landley @ 2007-10-17 23:09 UTC (permalink / raw)
To: Grant Likely
Cc: linuxppc-dev, Paul Mackerras, Milton Miller, Christoph Hellwig
In-Reply-To: <fa686aa40710171328y195aca9k6ee8a1a5502fcf57@mail.gmail.com>
On Wednesday 17 October 2007 3:28:41 pm Grant Likely wrote:
> > Including .dtbs in the kernel tree has a big practical problem:
> > they're binary, so can't be patch(1)ed, which makes updating them a
> > complete PITA.
I note that kconfig includes the lex/yacc output files (blah.c_shipped) so you
don't have to have lex and yacc installed to run "menuconfig". It _also_
includes the lex/yacc source which is what you patch and rebuild the _shipped
files from when they need to be changed.
> > I'm working on merging dtc into the kernel tree instead.
>
> I'm kind of late to this party; but I have to say I disagree. Most of
> us are doing just fine installing the dtc tool (and mkimage tool for
> that matter). Cloning it in the kernel tree is just asking for
> divergence.
Milton Miller has some patches that make a "PPC qemu target" kernel image
which doesn't include a device tree, and generates a rom image to boot qemu
with which contains a device tree and hands it off to the Linux kernel. If
qemu can merge that rom image in its BIOS collection, this approach would
meet my immediate needs.
> Cheers,
> g.
Rob
--
"One of my most productive days was throwing away 1000 lines of code."
- Ken Thompson.
^ permalink raw reply
* [PATCH] net: Add napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-17 22:04 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Roland Dreier, David S. Miller, Jeff Garzik,
linuxppc-dev list
In-Reply-To: <20071017083107.6a1b1bd3@freepuppy.rosehill>
net: Add __napi_synchronize() to sync with napi poll
The EMAC driver which needs to handle multiple devices with one
NAPI instance implements its own per-channel disable bit. However,
when setting such a bit, it needs to synchronize with the poller
(that is make sure that any pending poller instance has completed,
or is started late enough to see that disable bit).
This implements a low level __napi_synchronize() function to acheive
that. The underscores are to emphasis the low level aspect of it and
to discourage driver writers who don't know what they are doing to
use it (to please DaveM :-)
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
Back to msleep() since it fits my need well and that's what
you used as well. Note that the smp_mb() will turn into barrier()
on non-SMP.
I'll send a separate patch to fix EMAC to use the non __ version.
Index: linux-work/include/linux/netdevice.h
===================================================================
--- linux-work.orig/include/linux/netdevice.h 2007-10-17 13:31:32.000000000 +1000
+++ linux-work/include/linux/netdevice.h 2007-10-18 08:01:05.000000000 +1000
@@ -394,6 +394,23 @@ static inline void napi_disable(struct n
}
/**
+ * napi_synchronize - synchronize with a concurrent poll
+ * @n: napi context
+ *
+ * Synchronizes with a concurrent poll. Not to be used in normal
+ * drivers, mostly useful if you end up with multiple interfaces
+ * on one NAPI instance. This must be called from task context.
+ */
+static inline void napi_synchronize(struct napi_struct *n)
+{
+ smp_mb();
+#ifdef CONFIG_SMP
+ while (test_bit(NAPI_STATE_SCHED, &n->state))
+ msleep(1);
+#endif /* CONFIG_SMP */
+}
+
+/**
* napi_enable - enable NAPI scheduling
* @n: napi context
*
^ permalink raw reply
* Re: [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Benjamin Herrenschmidt @ 2007-10-17 21:52 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, Roland Dreier, David S. Miller, Jeff Garzik,
linuxppc-dev list
In-Reply-To: <20071017083107.6a1b1bd3@freepuppy.rosehill>
On Wed, 2007-10-17 at 08:31 -0700, Stephen Hemminger wrote:
> Please don't use double underscore, for this function name. There is no
> reason to not make it a normal API call.
>
> The sky2 fix I am working on will use napi_synchronize as well.
Allright. A compiler barrier in the !SMP case makes sense, but I would
still want an smp_mb() before the test_bit. I think it's a bug in
synchronize_irq not to have it.
Cheers,
Ben.
> --- a/include/linux/netdevice.h 2007-10-16 16:48:20.000000000 -0700
> +++ b/include/linux/netdevice.h 2007-10-17 08:29:55.000000000 -0700
> @@ -407,6 +407,24 @@ static inline void napi_enable(struct na
> clear_bit(NAPI_STATE_SCHED, &n->state);
> }
>
> +#ifdef CONFIG_SMP
> +/**
> + * napi_synchronize - wait until NAPI is not running
> + * @n: napi context
> + *
> + * Wait until NAPI is done being scheduled on this context.
> + * Any outstanding processing completes but
> + * does not disable future activations.
> + */
> +static inline void napi_synchronize(const struct napi_struct *n)
> +{
> + while (test_bit(NAPI_STATE_SCHED, &n->state))
> + msleep(1);
> +}
> +#else
> +# define napi_synchronize(n) barrier()
> +#endif
> +
> /*
> * The DEVICE structure.
> * Actually, this whole structure is a big mistake. It mixes I/O
^ permalink raw reply
* Re: Merge dtc
From: Jon Loeliger @ 2007-10-17 21:51 UTC (permalink / raw)
To: Linas Vepstas; +Cc: linuxppc-dev, Paul Mackerras, Timur Tabi, David Gibson
In-Reply-To: <20071017214217.GW4891@austin.ibm.com>
So, like, the other day Linas Vepstas mumbled:
>
> Isn't anyone concerned about the defacto fork-of-source-code that
> this causes?
You betcha. That's why I'm negative on the notion,
but I won't stand in front of _that_ train either.
> Which will be the official version?
The one I am maintaining here on jdl.com.
> How will the code baes be kept in sync?
I heard Gibson volunteer. :-)
jdl
^ permalink raw reply
* Re: Merge dtc
From: Linas Vepstas @ 2007-10-17 21:42 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, Paul Mackerras, David Gibson
In-Reply-To: <47166988.3010606@freescale.com>
On Wed, Oct 17, 2007 at 02:59:04PM -0500, Timur Tabi wrote:
> Kumar Gala wrote:
>
> > Just out of interest who's complaining? We don't include mkimage for
> > u-boot related builds and I haven't seen any gripes related to that.
>
> I think we should include mkimage *and* dtc. But then, I'm not sure how much
> weight my opinion has. :-)
Isn't anyone concerned about the defacto fork-of-source-code that
this causes? Which will be the official version? How will the code
baes be kept in sync?
--linas
^ permalink raw reply
* Re: [i2c] [PATCH take2] [POWERPC] i2c: adds support for i2c bus on 8xx
From: Jean Delvare @ 2007-10-17 21:00 UTC (permalink / raw)
To: Jochen Friedrich; +Cc: linuxppc-embedded, linux-kernel, i2c, Carsten Juttner
In-Reply-To: <47166085.1010608@scram.de>
On Wed, 17 Oct 2007 21:20:37 +0200, Jochen Friedrich wrote:
> >> + /* 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?
> >
>
> It's in fact 0x7F << 1. The same value is used in the 2.4 driver and
> in u-boot, as well. Slave operation is not supported.
I'm not sure what exactly you are doing here, but 0x7f isn't a valid
7-bit I2C address.
--
Jean Delvare
^ permalink raw reply
* Re: PPC440EPx GPIO control help
From: Josh Boyer @ 2007-10-17 20:41 UTC (permalink / raw)
To: Jeff Mock; +Cc: linuxppc-embedded
In-Reply-To: <47167002.7060202@mock.com>
On Wed, 17 Oct 2007 13:26:42 -0700
Jeff Mock <jeff@mock.com> wrote:
>
>
> Josh Boyer wrote:
> > On Tue, 2007-10-16 at 23:21 -0700, Jeff Mock wrote:
> >> David Hawkins wrote:
> >>>> I have a PPC440EPx Sequoia Evaluation board that runs on Linux 2.6.21.
> >>>> What I would want to do is to control (write and read values to) its
> >>>> GPIO. Perhaps similar to Turbo C's outputb(0x378,0x01) to write and
> >>>> inportb(0x378) to read. I read the PPC440EPx manual but I find it
> >>>> difficult to understand.
> >>>>
> >>>> Could anyone show me any tutorial or some sample codes?
> >>> I copied the code below from some test code I wrote for a TS7300
> >>> board (uses an ARM EP9302 processor). However, since its user-space
> >>> code it should work fine.
> >>>
> >> I might be a little out of date, but I think you must write your own
> >> driver to wiggle the GPIO pins on a 440 processor. I just finished a
> >> project using a 440GX with a 2.6.15 kernel (we froze the code about 8
> >> months ago).
> >>
> >> The 440 powerPC core is a 32-bit processor with 36-bit physical
> >> addresses. The physical address for the GPIO pins is someplace above
> >> 4GB. An mmap() of /dev/mem only lets you map the lower 4GB of the
> >> address space, as a result you can't write a user space program on the
> >> 440 to wiggle the GPIO pins. (This was true with 2.6.15, I can't speak
> >> for later kernels).
> >
> > This depends on the 440 chip itself. If I recall correctly, the
> > 440EP(x) chips don't have I/O above 4GB.
> >
>
> At first I thought you were right, I remember something about the 440GX
> being the weird processor with I/O mapped above 4GB. I just took a look
> at the 440EPx datasheet on the AMCC website and it looks like the GPIO
> registers are mapped above 4GB:
>
> GPIO0 controller 1 EF60 0B00
> GPIO1 controller 1 EF60 0C00
Actually, it's 440EP that has the addresses below 4GB, not 440EPx. My
apologies for the confusion. As far as "weird" goes in the 44x world,
440EP is the oddball as I believe most all other 44x versions have some
if not all I/O above 4GB.
josh
^ permalink raw reply
* Re: Merge dtc
From: Grant Likely @ 2007-10-17 20:31 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Mackerras, Timur Tabi, Paul, David Gibson
In-Reply-To: <20071017151230.01ccd2f8@weaponx.rchland.ibm.com>
On 10/17/07, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Wed, 17 Oct 2007 14:59:04 -0500
> Timur Tabi <timur@freescale.com> wrote:
>
> > Kumar Gala wrote:
> >
> > > Just out of interest who's complaining? We don't include mkimage for
> > > u-boot related builds and I haven't seen any gripes related to that.
> >
> > I think we should include mkimage *and* dtc. But then, I'm not sure how much
> > weight my opinion has. :-)
>
> Either way, if we're going to include DTC we need to do it soon.
> mkimage I'm less worried about because it should be a much smaller
> patch.
Cast my vote on the 'no' side. I don't like this approach.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: [PATCH 1/2] qemu platform, v2
From: Grant Likely @ 2007-10-17 20:28 UTC (permalink / raw)
To: Segher Boessenkool, Paul Mackerras, linuxppc-dev, Rob Landley,
Milton Miller, Christoph Hellwig
In-Reply-To: <20071001053326.GA15132@localhost.localdomain>
On 9/30/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Sep 28, 2007 at 06:53:28PM +0200, Segher Boessenkool wrote:
> > >> I'd be following this more closely if compiling a device tree didn't
> > >> currently
> > >> require an external utility (dtc or some such) that doesn't come with
> > >> the
> > >> Linux kernel. No other target platform I've built kernels for
> > >> requires such
> > >> an environmental dependency.
> > >
> > > No? You haven't built kernels for other platforms that have external
> > > dependencies such as perl, gcc, make, binutils, etc.? :)
> >
> > Two of the supported Linux archs cannot be built with a mainline
> > compiler, even!
> >
> > And I have to install GNU sed/awk to get builds to work, too.
> >
> > OTOH, it would be nice if we didn't need DTC -- it itself doesn't
> > build out-of-the-box on all systems, either ;-)
> >
> > >> (This is a problem both for hardwiring the
> > >> device tree into the kernel and for building a new boot rom from the
> > >> linux
> > >> kernel's ppc boot wrapper that would contain such a device tree to
> > >> feed to
> > >> the kernel.)
> > >
> > > It's only really been a problem for ps3 so far, since the embedded
> > > guys don't seem to have any difficulty with installing dtc. We are
> > > looking at what to do for ps3 and prep, and the answer may well
> > > involve bundling dtc in the kernel source (it's not too big, around
> > > 3400 lines).
> >
> > If only a few platforms have this problem, we could instead include
> > their .dtb files in the kernel source tree.
>
> Including .dtbs in the kernel tree has a big practical problem:
> they're binary, so can't be patch(1)ed, which makes updating them a
> complete PITA.
>
> I'm working on merging dtc into the kernel tree instead.
I'm kind of late to this party; but I have to say I disagree. Most of
us are doing just fine installing the dtc tool (and mkimage tool for
that matter). Cloning it in the kernel tree is just asking for
divergence.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195
^ permalink raw reply
* Re: PPC440EPx GPIO control help
From: Jeff Mock @ 2007-10-17 20:26 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-embedded
In-Reply-To: <1192618194.13993.25.camel@localhost.localdomain>
Josh Boyer wrote:
> On Tue, 2007-10-16 at 23:21 -0700, Jeff Mock wrote:
>> David Hawkins wrote:
>>>> I have a PPC440EPx Sequoia Evaluation board that runs on Linux 2.6.21.
>>>> What I would want to do is to control (write and read values to) its
>>>> GPIO. Perhaps similar to Turbo C's outputb(0x378,0x01) to write and
>>>> inportb(0x378) to read. I read the PPC440EPx manual but I find it
>>>> difficult to understand.
>>>>
>>>> Could anyone show me any tutorial or some sample codes?
>>> I copied the code below from some test code I wrote for a TS7300
>>> board (uses an ARM EP9302 processor). However, since its user-space
>>> code it should work fine.
>>>
>> I might be a little out of date, but I think you must write your own
>> driver to wiggle the GPIO pins on a 440 processor. I just finished a
>> project using a 440GX with a 2.6.15 kernel (we froze the code about 8
>> months ago).
>>
>> The 440 powerPC core is a 32-bit processor with 36-bit physical
>> addresses. The physical address for the GPIO pins is someplace above
>> 4GB. An mmap() of /dev/mem only lets you map the lower 4GB of the
>> address space, as a result you can't write a user space program on the
>> 440 to wiggle the GPIO pins. (This was true with 2.6.15, I can't speak
>> for later kernels).
>
> This depends on the 440 chip itself. If I recall correctly, the
> 440EP(x) chips don't have I/O above 4GB.
>
At first I thought you were right, I remember something about the 440GX
being the weird processor with I/O mapped above 4GB. I just took a look
at the 440EPx datasheet on the AMCC website and it looks like the GPIO
registers are mapped above 4GB:
GPIO0 controller 1 EF60 0B00
GPIO1 controller 1 EF60 0C00
Someone has to do the ioremap() to get at these physical addresses, so I
think you have to write a driver to get them mapped into user space.
It's unfortunate, especially for someone just getting started with the
processor that just wants to turn on an LED...
jeff
^ permalink raw reply
* Re: loading NOR flash from DT
From: Sergei Shtylyov @ 2007-10-17 20:23 UTC (permalink / raw)
To: Sebastian Siewior; +Cc: linuxppc-embedded
In-Reply-To: <20071017195542.GA5501@Chamillionaire.breakpoint.cc>
Sebastian Siewior wrote:
>>>I have here a MPC8544 DS board with NOR flash on it and Kumar's git
>>>tree. I'm trying to detect this flash with the physmap_of module. I
>>>added a nor_flash block (I used sequoia.dts as an example) into my
>>>device tree on the same level as soc8544 or memory (or between them).
>>>After modprobing physmap_of nothing happend. Then I tried to make a tree
>>>with plb -> opb -> ebc and finally nor_flash but still nothing changed.
>>>
>>>Is there a user space dependency or did I just edit my device tree the
>>>wrong way? Any hints are welcome :)
>>
>>Post your device tree please. I don't recall MPC8544 even having PLB,
>>OPB, or EBC busses, so they likely aren't getting probed.
[...]
> flash@fc000000 {
> compatible = "amd,s29gl512n", "cfi-flash";
> reg = <fc000000 4000000>;
> bank-width = <2>;
> #address-cells = <1>;
> #size-cells = <1>;
> fs@0 {
> label = "fs";
> reg = <0 f80000>;
> };
> firmware@f80000 {
> label ="firmware";
> reg = <f80000 80000>;
> read-only;
> };
This looks good, so the problem is it doesn't get registered by the
platform code as I've said.
WBR, Sergei
^ permalink raw reply
* Re: loading NOR flash from DT
From: Scott Wood @ 2007-10-17 20:17 UTC (permalink / raw)
To: Sebastian Siewior; +Cc: linuxppc-embedded
In-Reply-To: <20071017195542.GA5501@Chamillionaire.breakpoint.cc>
Sebastian Siewior wrote:
> flash@fc000000 {
It's probably not getting probed, because it's not under a probed bus.
Try adding the root node to the list passed to of_platform_bus_probe().
> compatible = "amd,s29gl512n", "cfi-flash";
> reg = <fc000000 4000000>;
> bank-width = <2>;
> #address-cells = <1>;
> #size-cells = <1>;
> fs@0 {
> label = "fs";
> reg = <0 f80000>;
> };
> firmware@f80000 {
> label ="firmware";
> reg = <f80000 80000>;
> read-only;
> };
>
> soc8544@e0000000 {
Missing }; for the flash node.
-Scott
^ permalink raw reply
* Re: Merge dtc
From: Josh Boyer @ 2007-10-17 20:12 UTC (permalink / raw)
To: Timur Tabi; +Cc: linuxppc-dev, Paul, Mackerras, David Gibson
In-Reply-To: <47166988.3010606@freescale.com>
On Wed, 17 Oct 2007 14:59:04 -0500
Timur Tabi <timur@freescale.com> wrote:
> Kumar Gala wrote:
>
> > Just out of interest who's complaining? We don't include mkimage for
> > u-boot related builds and I haven't seen any gripes related to that.
>
> I think we should include mkimage *and* dtc. But then, I'm not sure how much
> weight my opinion has. :-)
Either way, if we're going to include DTC we need to do it soon.
mkimage I'm less worried about because it should be a much smaller
patch.
josh
^ permalink raw reply
* Re: loading NOR flash from DT
From: Sergei Shtylyov @ 2007-10-17 19:54 UTC (permalink / raw)
To: Sebastian Siewior; +Cc: linuxppc-embedded
In-Reply-To: <20071017192651.GA5192@Chamillionaire.breakpoint.cc>
Hello.
Sebastian Siewior wrote:
> I have here a MPC8544 DS board with NOR flash on it and Kumar's git
> tree. I'm trying to detect this flash with the physmap_of module. I
> added a nor_flash block (I used sequoia.dts as an example) into my
> device tree on the same level as soc8544 or memory (or between them).
I guess you were using quite recent kernel version where the file that
published the node (arch/powerpc/sysdev/rom.c) had been already dremoved and
and the platform code still didn't publish the on-board devices via
of_platform_bus_probe().
> After modprobing physmap_of nothing happend. Then I tried to make a tree
> with plb -> opb -> ebc and finally nor_flash but still nothing changed.
Add either of_platform_bus_probe() or platform_device_create() to
arch/powerpc/platforms/85xx/85xx_ds.c.
> Is there a user space dependency or did I just edit my device tree the
> wrong way? Any hints are welcome :)
You haven't cited your node, so no conclusion acn be made about it...
WBR, Sergei
^ permalink raw reply
* Re: Merge dtc
From: Timur Tabi @ 2007-10-17 19:59 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Paul Mackerras, David Gibson
In-Reply-To: <5E3B6FCC-5930-4C76-AB4B-86B35551B9E0@kernel.crashing.org>
Kumar Gala wrote:
> Just out of interest who's complaining? We don't include mkimage for
> u-boot related builds and I haven't seen any gripes related to that.
I think we should include mkimage *and* dtc. But then, I'm not sure how much
weight my opinion has. :-)
--
Timur Tabi
Linux Kernel Developer @ Freescale
^ permalink raw reply
* [PATCH][NET] gianfar: fix obviously wrong #ifdef CONFIG_GFAR_NAPI placement
From: Anton Vorontsov @ 2007-10-17 19:57 UTC (permalink / raw)
To: leoli, jgarzik, paulus; +Cc: netdev, linux-kernel, linuxppc-dev
Erroneous #ifdef introduced by 293c8513398657f6263fcdb03c87f2760cf61be4
causing NAPI-less ethernet malfunctioning.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Vitaly Bordug <vbordug@ru.mvista.com>
---
if (...)
#if
...;
#endif
good candidate for checkpatch?
drivers/net/gianfar.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index cc288d8..38268d7 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -956,10 +956,12 @@ static int gfar_enet_open(struct net_device *dev)
}
err = startup_gfar(dev);
- if (err)
+ if (err) {
#ifdef CONFIG_GFAR_NAPI
napi_disable(&priv->napi);
#endif
+ return err;
+ }
netif_start_queue(dev);
--
1.5.0.6
^ permalink raw reply related
* Re: loading NOR flash from DT
From: Sebastian Siewior @ 2007-10-17 19:55 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-embedded
In-Reply-To: <20071017144759.320209bd@weaponx.rchland.ibm.com>
* Josh Boyer | 2007-10-17 14:47:59 [-0500]:
>On Wed, 17 Oct 2007 21:26:51 +0200
>Sebastian Siewior <linuxppc-embedded@ml.breakpoint.cc> wrote:
>
>> Hello,
>>
>> I have here a MPC8544 DS board with NOR flash on it and Kumar's git
>> tree. I'm trying to detect this flash with the physmap_of module. I
>> added a nor_flash block (I used sequoia.dts as an example) into my
>> device tree on the same level as soc8544 or memory (or between them).
>> After modprobing physmap_of nothing happend. Then I tried to make a tree
>> with plb -> opb -> ebc and finally nor_flash but still nothing changed.
>>
>> Is there a user space dependency or did I just edit my device tree the
>> wrong way? Any hints are welcome :)
>
>Post your device tree please. I don't recall MPC8544 even having PLB,
>OPB, or EBC busses, so they likely aren't getting probed.
/*
* MPC8544 DS Device Tree Source
*
* Copyright 2007 Freescale Semiconductor Inc.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
* option) any later version.
*/
/ {
model = "MPC8544DS";
compatible = "MPC8544DS", "MPC85xxDS";
#address-cells = <1>;
#size-cells = <1>;
cpus {
#cpus = <1>;
#address-cells = <1>;
#size-cells = <0>;
PowerPC,8544@0 {
device_type = "cpu";
reg = <0>;
d-cache-line-size = <20>; // 32 bytes
i-cache-line-size = <20>; // 32 bytes
d-cache-size = <8000>; // L1, 32K
i-cache-size = <8000>; // L1, 32K
timebase-frequency = <0>;
bus-frequency = <0>;
clock-frequency = <0>;
};
};
memory {
device_type = "memory";
reg = <00000000 00000000>; // Filled by U-Boot
};
flash@fc000000 {
compatible = "amd,s29gl512n", "cfi-flash";
reg = <fc000000 4000000>;
bank-width = <2>;
#address-cells = <1>;
#size-cells = <1>;
fs@0 {
label = "fs";
reg = <0 f80000>;
};
firmware@f80000 {
label ="firmware";
reg = <f80000 80000>;
read-only;
};
soc8544@e0000000 {
#address-cells = <1>;
#size-cells = <1>;
device_type = "soc";
ranges = <00000000 e0000000 00100000>;
reg = <e0000000 00001000>; // CCSRBAR 1M
bus-frequency = <0>; // Filled out by uboot.
memory-controller@2000 {
compatible = "fsl,8544-memory-controller";
reg = <2000 1000>;
interrupt-parent = <&mpic>;
interrupts = <12 2>;
};
l2-cache-controller@20000 {
compatible = "fsl,8544-l2-cache-controller";
reg = <20000 1000>;
cache-line-size = <20>; // 32 bytes
cache-size = <40000>; // L2, 256K
interrupt-parent = <&mpic>;
interrupts = <10 2>;
};
i2c@3000 {
device_type = "i2c";
compatible = "fsl-i2c";
reg = <3000 100>;
interrupts = <2b 2>;
interrupt-parent = <&mpic>;
dfsrr;
};
mdio@24520 {
#address-cells = <1>;
#size-cells = <0>;
device_type = "mdio";
compatible = "gianfar";
reg = <24520 20>;
phy0: ethernet-phy@0 {
interrupt-parent = <&mpic>;
interrupts = <a 1>;
reg = <0>;
device_type = "ethernet-phy";
};
phy1: ethernet-phy@1 {
interrupt-parent = <&mpic>;
interrupts = <a 1>;
reg = <1>;
device_type = "ethernet-phy";
};
};
ethernet@24000 {
#address-cells = <1>;
#size-cells = <0>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <24000 1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <1d 2 1e 2 22 2>;
interrupt-parent = <&mpic>;
phy-handle = <&phy0>;
phy-connection-type = "rgmii-id";
};
ethernet@26000 {
#address-cells = <1>;
#size-cells = <0>;
device_type = "network";
model = "TSEC";
compatible = "gianfar";
reg = <26000 1000>;
local-mac-address = [ 00 00 00 00 00 00 ];
interrupts = <1f 2 20 2 21 2>;
interrupt-parent = <&mpic>;
phy-handle = <&phy1>;
phy-connection-type = "rgmii-id";
};
serial@4500 {
device_type = "serial";
compatible = "ns16550";
reg = <4500 100>;
clock-frequency = <0>;
interrupts = <2a 2>;
interrupt-parent = <&mpic>;
};
serial@4600 {
device_type = "serial";
compatible = "ns16550";
reg = <4600 100>;
clock-frequency = <0>;
interrupts = <2a 2>;
interrupt-parent = <&mpic>;
};
global-utilities@e0000 { //global utilities block
compatible = "fsl,mpc8548-guts";
reg = <e0000 1000>;
fsl,has-rstcr;
};
mpic: pic@40000 {
clock-frequency = <0>;
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <2>;
reg = <40000 40000>;
compatible = "chrp,open-pic";
device_type = "open-pic";
big-endian;
};
};
pci@e0008000 {
compatible = "fsl,mpc8540-pci";
device_type = "pci";
interrupt-map-mask = <f800 0 0 7>;
interrupt-map = <
/* IDSEL 0x11 J17 Slot 1 */
8800 0 0 1 &mpic 2 1
8800 0 0 2 &mpic 3 1
8800 0 0 3 &mpic 4 1
8800 0 0 4 &mpic 1 1
/* IDSEL 0x12 J16 Slot 2 */
9000 0 0 1 &mpic 3 1
9000 0 0 2 &mpic 4 1
9000 0 0 3 &mpic 2 1
9000 0 0 4 &mpic 1 1>;
interrupt-parent = <&mpic>;
interrupts = <18 2>;
bus-range = <0 ff>;
ranges = <02000000 0 c0000000 c0000000 0 20000000
01000000 0 00000000 e1000000 0 00010000>;
clock-frequency = <3f940aa>;
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
reg = <e0008000 1000>;
};
pcie@e0009000 {
compatible = "fsl,mpc8548-pcie";
device_type = "pci";
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
reg = <e0009000 1000>;
bus-range = <0 ff>;
ranges = <02000000 0 80000000 80000000 0 20000000
01000000 0 00000000 e1010000 0 00010000>;
clock-frequency = <1fca055>;
interrupt-parent = <&mpic>;
interrupts = <1a 2>;
interrupt-map-mask = <f800 0 0 7>;
interrupt-map = <
/* IDSEL 0x0 */
0000 0 0 1 &mpic 4 1
0000 0 0 2 &mpic 5 1
0000 0 0 3 &mpic 6 1
0000 0 0 4 &mpic 7 1
>;
pcie@0 {
reg = <0 0 0 0 0>;
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
ranges = <02000000 0 80000000
02000000 0 80000000
0 20000000
01000000 0 00000000
01000000 0 00000000
0 00010000>;
};
};
pcie@e000a000 {
compatible = "fsl,mpc8548-pcie";
device_type = "pci";
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
reg = <e000a000 1000>;
bus-range = <0 ff>;
ranges = <02000000 0 a0000000 a0000000 0 10000000
01000000 0 00000000 e1020000 0 00010000>;
clock-frequency = <1fca055>;
interrupt-parent = <&mpic>;
interrupts = <19 2>;
interrupt-map-mask = <f800 0 0 7>;
interrupt-map = <
/* IDSEL 0x0 */
0000 0 0 1 &mpic 0 1
0000 0 0 2 &mpic 1 1
0000 0 0 3 &mpic 2 1
0000 0 0 4 &mpic 3 1
>;
pcie@0 {
reg = <0 0 0 0 0>;
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
ranges = <02000000 0 a0000000
02000000 0 a0000000
0 10000000
01000000 0 00000000
01000000 0 00000000
0 00010000>;
};
};
pcie@e000b000 {
compatible = "fsl,mpc8548-pcie";
device_type = "pci";
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
reg = <e000b000 1000>;
bus-range = <0 ff>;
ranges = <02000000 0 b0000000 b0000000 0 00100000
01000000 0 00000000 b0100000 0 00100000>;
clock-frequency = <1fca055>;
interrupt-parent = <&mpic>;
interrupts = <1b 2>;
interrupt-map-mask = <fb00 0 0 0>;
interrupt-map = <
// IDSEL 0x1c USB
e000 0 0 0 &i8259 c 2
e100 0 0 0 &i8259 9 2
e200 0 0 0 &i8259 a 2
e300 0 0 0 &i8259 b 2
// IDSEL 0x1d Audio
e800 0 0 0 &i8259 6 2
// IDSEL 0x1e Legacy
f000 0 0 0 &i8259 7 2
f100 0 0 0 &i8259 7 2
// IDSEL 0x1f IDE/SATA
f800 0 0 0 &i8259 e 2
f900 0 0 0 &i8259 5 2
>;
pcie@0 {
reg = <0 0 0 0 0>;
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";
ranges = <02000000 0 b0000000
02000000 0 b0000000
0 00100000
01000000 0 00000000
01000000 0 00000000
0 00100000>;
uli1575@0 {
reg = <0 0 0 0 0>;
#size-cells = <2>;
#address-cells = <3>;
ranges = <02000000 0 b0000000
02000000 0 b0000000
0 00100000
01000000 0 00000000
01000000 0 00000000
0 00100000>;
isa@1e {
device_type = "isa";
#interrupt-cells = <2>;
#size-cells = <1>;
#address-cells = <2>;
reg = <f000 0 0 0 0>;
ranges = <1 0
01000000 0 0
00001000>;
interrupt-parent = <&i8259>;
i8259: interrupt-controller@20 {
reg = <1 20 2
1 a0 2
1 4d0 2>;
interrupt-controller;
device_type = "interrupt-controller";
#address-cells = <0>;
#interrupt-cells = <2>;
compatible = "chrp,iic";
interrupts = <9 2>;
interrupt-parent = <&mpic>;
};
i8042@60 {
#size-cells = <0>;
#address-cells = <1>;
reg = <1 60 1 1 64 1>;
interrupts = <1 3 c 3>;
interrupt-parent = <&i8259>;
keyboard@0 {
reg = <0>;
compatible = "pnpPNP,303";
};
mouse@1 {
reg = <1>;
compatible = "pnpPNP,f03";
};
};
rtc@70 {
compatible = "pnpPNP,b00";
reg = <1 70 2>;
};
gpio@400 {
reg = <1 400 80>;
};
};
};
};
};
};
>
>josh
Sebastian
^ permalink raw reply
* Re: loading NOR flash from DT
From: Josh Boyer @ 2007-10-17 19:47 UTC (permalink / raw)
To: Sebastian Siewior; +Cc: linuxppc-embedded
In-Reply-To: <20071017192651.GA5192@Chamillionaire.breakpoint.cc>
On Wed, 17 Oct 2007 21:26:51 +0200
Sebastian Siewior <linuxppc-embedded@ml.breakpoint.cc> wrote:
> Hello,
>
> I have here a MPC8544 DS board with NOR flash on it and Kumar's git
> tree. I'm trying to detect this flash with the physmap_of module. I
> added a nor_flash block (I used sequoia.dts as an example) into my
> device tree on the same level as soc8544 or memory (or between them).
> After modprobing physmap_of nothing happend. Then I tried to make a tree
> with plb -> opb -> ebc and finally nor_flash but still nothing changed.
>
> Is there a user space dependency or did I just edit my device tree the
> wrong way? Any hints are welcome :)
Post your device tree please. I don't recall MPC8544 even having PLB,
OPB, or EBC busses, so they likely aren't getting probed.
josh
^ permalink raw reply
* Re: [PATCH take2] [POWERPC] i2c: adds support for i2c bus on 8xx
From: Scott Wood @ 2007-10-17 19:35 UTC (permalink / raw)
To: Jochen Friedrich
Cc: Carsten Juttner, linux-kernel, i2c, linuxppc-embedded@ozlabs.org
In-Reply-To: <47166085.1010608@scram.de>
Jochen Friedrich wrote:
>>> 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.
>>
>
> Why? Documentation/powerpc/booting-without-of.txt says for I2C interfaces
> device_type is required and should be "i2c". Is this no longer true?
booting-without-of.txt should be changed.
>> 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.
>
> CPM2 i2c seems to be the same. However, i have no way to test this.
OK, let's make the compatible "fsl,mpc885-i2c", "fsl,cpm1-i2c",
"fsl,cpm-i2c".
For now, match on the last one, but if any differences pop up, we have
the more specific ones.
> I noticed cpm1_set_pin32, but this function don't seem to set the
> odr register. Will this be added? Then it would be:
>
> {CPM_PORTB, 26, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},
> {CPM_PORTB, 27, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},
>
Ah, missed that -- there's opendrain support for port E, but I missed
that port B had it as well. Feel free to add it.
>> It's a 7-bit address... and are you sure that 0x7e is unique? Does this
>> driver even support slave operation?
>
> It's in fact 0x7F << 1.
Gah, I hate powerpc bit numbering, especially without the
numbered-as-if-64-bit hack. I specifically looked at the manual before
to see if it was shifted, saw "0-6", and concluded it wasn't. :-P
> The same value is used in the 2.4 driver and
> in u-boot, as well.
That doesn't mean that this isn't a good time to review what the code is
doing. :-)
> Slave operation is not supported.
If slave operation isn't supported, how is this value used?
>> Why is an 8xx driver matching all i2c cpm (i.e. what about cpm2)?
>
> With the suggested change to use fsl,cpm-command, the driver should
> be able to use both cpm1 and cpm2. The operation and structs for i2c
> are identical. The only difference might be the hack^wsupport for
> relocation.
OK. Would that allow it to become one driver, rather than a wrapper and
an algorithm?
-Scott
^ permalink raw reply
* loading NOR flash from DT
From: Sebastian Siewior @ 2007-10-17 19:26 UTC (permalink / raw)
To: linuxppc-embedded
Hello,
I have here a MPC8544 DS board with NOR flash on it and Kumar's git
tree. I'm trying to detect this flash with the physmap_of module. I
added a nor_flash block (I used sequoia.dts as an example) into my
device tree on the same level as soc8544 or memory (or between them).
After modprobing physmap_of nothing happend. Then I tried to make a tree
with plb -> opb -> ebc and finally nor_flash but still nothing changed.
Is there a user space dependency or did I just edit my device tree the
wrong way? Any hints are welcome :)
Sebastian
^ permalink raw reply
* Re: [PATCH take2] [POWERPC] i2c: adds support for i2c bus on 8xx
From: Jochen Friedrich @ 2007-10-17 19:20 UTC (permalink / raw)
To: Scott Wood
Cc: Carsten Juttner, linux-kernel, i2c, linuxppc-embedded@ozlabs.org
In-Reply-To: <20071015183107.GA4361@loki.buserror.net>
Hi Scott,
> 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.
>
OK. I'll remove the ARC=ppc parts.
>> 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.
>
Why? Documentation/powerpc/booting-without-of.txt says for I2C interfaces
device_type is required and should be "i2c". Is this no longer true?
> 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.
>
CPM2 i2c seems to be the same. However, i have no way to test this.
>> +#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},
>
I noticed cpm1_set_pin32, but this function don't seem to set the
odr register. Will this be added? Then it would be:
{CPM_PORTB, 26, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},
{CPM_PORTB, 27, CPM_PIN_OUTPUT | CPM_PIN_OPENDRAIN},
>> + /* 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?
>
It's in fact 0x7F << 1. The same value is used in the 2.4 driver and
in u-boot, as well. Slave operation is not supported.
> Why is an 8xx driver matching all i2c cpm (i.e. what about cpm2)?
>
With the suggested change to use fsl,cpm-command, the driver should
be able to use both cpm1 and cpm2. The operation and structs for i2c
are identical. The only difference might be the hack^wsupport for
relocation.
Thanks,
Jochen
^ 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