Re: [PATCH RFC] pata_platform: add 8 bit data io support
Tejun Heo wrote: Hello, Wang Jian wrote: +static void pata_platform_postreset(struct ata_link *link, unsigned int *classes) +{ + struct ata_port *ap = link-ap; + struct ata_device *dev; + u8 select = ATA_DEVICE_OBS; + + /* Call default callback first */ + ata_sff_postreset(link, classes); + + if (!(ap-flags ATA_FLAG_8BIT_DATA)) + return; + + /* Set 8-bit mode. We know we can do that */ + ata_link_for_each_dev(dev, link) { + if (dev-devno) + select |= ATA_DEV1; + + iowrite8(SETFEATURES_8BIT_ON, ap-ioaddr.feature_addr); + iowrite8(select, ap-ioaddr.device_addr); + iowrite8(ATA_CMD_SET_FEATURES, ap-ioaddr.command_addr); Aieee... Please don't do this. I think it best belongs to ata_dev_configure() or -dev_config() if you wanna put it in low level driver. Good. I remember the spec states that this setfeature command should be issued every time reset is issued. This is just a quick and safe hack. I will look into libata deeper and figure out how to do it better per your suggestion. @@ -106,7 +159,8 @@ int __devinit __pata_platform_probe(struct device *dev, struct resource *ctl_res, struct resource *irq_res, unsigned int ioport_shift, - int __pio_mask) + int __pio_mask, + unsigned int data_width) { struct ata_host *host; struct ata_port *ap; @@ -140,6 +194,9 @@ int __devinit __pata_platform_probe(struct device *dev, ap-pio_mask = __pio_mask; ap-flags |= ATA_FLAG_SLAVE_POSS; + if (data_width == ATA_DATA_WIDTH_8BIT) + ap-flags |= ATA_FLAG_8BIT_DATA; It's strange to define ATA_DATA_WIDTH_* constants in ata.h and only use it in ata_platform. I have expressed in another reply that the best place the code belongs to should be decided first. The usage of flags looks ugly too :) Overall, I think the bulk of the 8bit PIO implementation should go into the libata core layer and transfer width should be property of struct ata_device - probably right above or below pio/dma_mode and xfer_mode/shift fields. Yes, I agree it'd better go into libata core layer. But for transfer width, I think it is not belongs to ata_device. It's about how ata controller wired for data line. (In my case, it is how CF card wired). Am I wrong? Best regards ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH RFC] pata_platform: add 8 bit data io support
To avoid adding another rare used ata_port member, new bit is added to ata_port-flags. Originally, I hacked pata_platform to make it 8bit only to support 8bit data wired CF card. This patch is more generic. With this patch, __pata_platform_probe() interface is changed, and pata_of_platform is broken, so a small patch is needed. Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/ata/pata_platform.c | 63 -- include/linux/ata.h |8 + include/linux/ata_platform.h |4 ++ include/linux/libata.h |1 + 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..d2276ad 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -50,9 +50,62 @@ static struct scsi_host_template pata_platform_sht = { ATA_PIO_SHT(DRV_NAME), }; +static void pata_platform_postreset(struct ata_link *link, unsigned int *classes) +{ + struct ata_port *ap = link-ap; + struct ata_device *dev; + u8 select = ATA_DEVICE_OBS; + + /* Call default callback first */ + ata_sff_postreset(link, classes); + + if (!(ap-flags ATA_FLAG_8BIT_DATA)) + return; + + /* Set 8-bit mode. We know we can do that */ + ata_link_for_each_dev(dev, link) { + if (dev-devno) + select |= ATA_DEV1; + + iowrite8(SETFEATURES_8BIT_ON, ap-ioaddr.feature_addr); + iowrite8(select, ap-ioaddr.device_addr); + iowrite8(ATA_CMD_SET_FEATURES, ap-ioaddr.command_addr); + } +} + +static unsigned int pata_platform_data_xfer(struct ata_device *dev, + unsigned char *buf, unsigned int buflen, int rw) +{ + struct ata_port *ap = dev-link-ap; + + if (!(ap-flags ATA_FLAG_8BIT_DATA)) + return ata_sff_data_xfer(dev, buf, buflen, rw); + + if (rw == READ) + ioread8_rep(ap-ioaddr.data_addr, buf, buflen); + else + iowrite8_rep(ap-ioaddr.data_addr, buf, buflen); + + return buflen; +} + +static unsigned int pata_platform_data_xfer_noirq(struct ata_device *dev, + unsigned char *buf, unsigned int buflen, int rw) +{ + unsigned long flags; + unsigned int consumed; + + local_irq_save(flags); + consumed = pata_platform_data_xfer(dev, buf, buflen, rw); + local_irq_restore(flags); + + return consumed; +} + static struct ata_port_operations pata_platform_port_ops = { .inherits = ata_sff_port_ops, - .sff_data_xfer = ata_sff_data_xfer_noirq, + .postreset = pata_platform_postreset, + .sff_data_xfer = pata_platform_data_xfer_noirq, .cable_detect = ata_cable_unknown, .set_mode = pata_platform_set_mode, .port_start = ATA_OP_NULL, @@ -106,7 +159,8 @@ int __devinit __pata_platform_probe(struct device *dev, struct resource *ctl_res, struct resource *irq_res, unsigned int ioport_shift, - int __pio_mask) + int __pio_mask, + unsigned int data_width) { struct ata_host *host; struct ata_port *ap; @@ -140,6 +194,9 @@ int __devinit __pata_platform_probe(struct device *dev, ap-pio_mask = __pio_mask; ap-flags |= ATA_FLAG_SLAVE_POSS; + if (data_width == ATA_DATA_WIDTH_8BIT) + ap-flags |= ATA_FLAG_8BIT_DATA; + /* * Use polling mode if there's no IRQ */ @@ -242,7 +299,7 @@ static int __devinit pata_platform_probe(struct platform_device *pdev) return __pata_platform_probe(pdev-dev, io_res, ctl_res, irq_res, pp_info ? pp_info-ioport_shift : 0, -pio_mask); +pio_mask, pp_info-data_width); } static int __devexit pata_platform_remove(struct platform_device *pdev) diff --git a/include/linux/ata.h b/include/linux/ata.h index be00973..4ce26df 100644 --- a/include/linux/ata.h +++ b/include/linux/ata.h @@ -45,6 +45,11 @@ enum { ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */ ATA_MAX_SECTORS_TAPE= 65535, + ATA_DATA_WIDTH_8BIT = 1, + ATA_DATA_WIDTH_16BIT= 2, + ATA_DATA_WIDTH_DEFAULT = 2, + ATA_DATA_WIDTH_32BIT= 4, + ATA_ID_WORDS= 256, ATA_ID_CONFIG = 0, ATA_ID_CYLS = 1, @@ -280,6 +285,9 @@ enum { XFER_PIO_0 = 0x08, XFER_PIO_SLOW = 0x00, + SETFEATURES_8BIT_ON = 0x01, /* Enable 8 bit data transfers */ + SETFEATURES_8BIT_OFF= 0x81, /* Disable 8 bit data transfers
Re: [PATCH RFC] pata_platform: add 8 bit data io support
Benjamin Herrenschmidt 写道: On Sun, 2008-10-12 at 02:00 +0800, Wang Jian wrote: To avoid adding another rare used ata_port member, new bit is added to ata_port-flags. Originally, I hacked pata_platform to make it 8bit only to support 8bit data wired CF card. This patch is more generic. With this patch, __pata_platform_probe() interface is changed, and pata_of_platform is broken, so a small patch is needed. Signed-off-by: Wang Jian [EMAIL PROTECTED] --- A couple of things. First I would personally prefer (but I'm not the libata maintainer so it's up to Jeff ...) if you had a separate patch that adds the 8-bit support to libata core first, and then a patch that modifies pata_platform. I will do that if my 8-bit mode patch is done right technically. Then, in order to avoid breaking bisection, I would like you to fixup pata_of_platform in the same patch that modifies __pata_platform_probe so there is no breakage in between patches. Yes. Now, regarding the patch itself, if the core grows a 8-bit flag, then I strongly suspect the core should also grow the 8-bit xfer function rather than having it hidden in pata_platform. This is the main reason I send a single RFC patch. Where to add 8-bit mode should be decided first. Because 8-bit mode is mostly used for embedded devices, my opinion is 8-bit mode in pata_platform is enough. However, look at pata_platform_data_xfer() I added, the code can be merged into ata_sff_data_xfer() of libata-sff.c easily. Moving the code there is trivial if necessary. Another problem should be addressed: using flags v.s. using data_width member. I add a bit to indicate 8 bit mode, but this seems to be a problem for future 32 bit I/O support in libata. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Compact Flash on 8349mITX
Jeff Borlin wrote: Anton Vorontsov-2 wrote: This happens just before the PATA information is printed. I'm not libata expert; and from the brief look I don't see where libata clears any pending unexpected irqs. Just a guesswork, could you try this patch? This patch did not appear to change anything. Without this patch, you can't use it without irq. Anton Vorontsov-2 wrote: Can you check if the CF will work w/o IRQs? To try it: Your directions did result in progress: [...] scsi4 : pata_platform ata5: PATA max PIO6 no IRQ, using PIO polling mmio cmd 0xf000 ctl 0xf20c ata5.00: CFA: SanDisk SDCFJ-512, HDX 3.13, max MWDMA2 ata5.00: 1000944 sectors, multi 0: LBA ata5.00: configured for PIO scsi 4:0:0:0: Direct-Access ATA SanDisk SDCFJ-51 HDX PQ: 0 ANSI: 5 sd 4:0:0:0: [sda] 1000944 512-byte hardware sectors (512 MB) sd 4:0:0:0: [sda] Write Protect is off sd 4:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA sd 4:0:0:0: [sda] 1000944 512-byte hardware sectors (512 MB) sd 4:0:0:0: [sda] Write Protect is off sd 4:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA sda: sda1 sda2 sd 4:0:0:0: [sda] Attached SCSI removable disk sd 4:0:0:0: Attached scsi generic sg0 type 0 physmap platform flash device: 0100 at fe00 [...] / # dd if=/dev/sda of=/dev/null count=2 2+0 records in 2+0 records out / # mount -t vfat /dev/sda /mnt/cf mount: Mounting /dev/sda on /mnt/cf failed: Invalid argument / # mount -t vfat /dev/sda2 /mnt/cf mount: Mounting /dev/sda2 on /mnt/cf failed: Invalid argument I still need to figure out what I may be doing wrong here, but wanted to post the results I have so far in case they are useful. The CF card seems to work fine. See if your kernel supports vfat by # cat /proc/filesystems Of course, you need to mount /proc properly first. If your kernel doesn't support vfat, make it support. Or you can insert a blank CF card and play with it (ext2, etc) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Compact Flash on 8349mITX
How 8349mITX's compact flash is wired? If it is wired using 8 bit data bus line, without another patch, data transfer can't be done. Looking at logs Sam Sparks provided (without irq): ata1.00: failed to IDENTIFY (I/O error, err_mask=0x4) it seems data transfer failed. I have a 8 bit data transfer mode dirty hack on pata_platform. I am waiting for the no irq handling patch goes in and then make my patch more generic. Anton Vorontsov wrote: On Thu, Oct 09, 2008 at 08:52:09AM -0700, Jeff Borlin wrote: I have taken over this effort to get Compact Flash working on the 8349mITX board and am running into these same issues. I can get uBoot to list the contents of a CF card, U-Boot doesn't use interrupts. but am running into a couple problems through the kernel. There appears to be an IRQ problem and a general communication problem with the CF card, possibly related. Note that I am very new to Linux so please don't assume obvious things are correct I am using the latest from /linux/kernel/git/benh/powerpc.git, and created my dtb from: arch/powerpc/dts/mpc8349emitx.dts. Any ideas on what I could try or look into next with either issue? boot info: [...] [...] This happens just before the PATA information is printed. I'm not libata expert; and from the brief look I don't see where libata clears any pending unexpected irqs. Just a guesswork, could you try this patch? diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..3b7c79d 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -15,6 +15,7 @@ #include linux/module.h #include linux/init.h #include linux/blkdev.h +#include linux/io.h #include scsi/scsi_host.h #include linux/ata.h #include linux/libata.h @@ -171,6 +172,8 @@ int __devinit __pata_platform_probe(struct device *dev, pata_platform_setup_port(ap-ioaddr, ioport_shift); + ioread8(ap-ioaddr.status_addr); + ata_port_desc(ap, %s cmd 0x%llx ctl 0x%llx, mmio ? mmio : ioport, (unsigned long long)io_res-start, (unsigned long long)ctl_res-start); irq 23: nobody cared (try booting with the irqpoll option) Call Trace: [cec29a30] [c00089a4] 0xc00089a4 (unreliable) [cec29a60] [c00467d8] 0xc00467d8 [...] Disabling IRQ #23 scsi4 : pata_platform ata5: PATA max PIO6 mmio cmd 0xf000 ctl 0xf20c irq 23 ata5.00: CFA: SanDisk SDCFJ-512, HDX 3.13, max MWDMA2 ata5.00: 1000944 sectors, multi 0: LBA ata5.00: configured for PIO Can you check if the CF will work w/o IRQs? To try it: 1. open the arch/powerpc/dts/mpc8349emitx.dts file, find the pata node: [EMAIL PROTECTED],0 { compatible = fsl,mpc8349emitx-pata, ata-generic; reg = 0x3 0x0 0x10 0x3 0x20c 0x4; reg-shift = 1; pio-mode = 6; interrupts = 23 0x8; interrupt-parent = ipic; }; 2. delete interrupts = ; and interrupt-parent = ; lines. 3. recompile the dts to dtb. 4. Apply this patch to the kernel http://lkml.org/lkml/2008/10/6/176 5. And try to boot the patched kernel using the re-compiled dtb file. scsi 4:0:0:0: Direct-Access ATA SanDisk SDCFJ-51 HDX PQ: 0 ANSI: 5 sd 4:0:0:0: [sda] 1000944 512-byte hardware sectors (512 MB) sd 4:0:0:0: [sda] Write Protect is off sd 4:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA sd 4:0:0:0: [sda] 1000944 512-byte hardware sectors (512 MB) sd 4:0:0:0: [sda] Write Protect is off sd 4:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA sda:3ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen ata5.00: cmd 20/00:08:00:00:00/00:00:00:00:00/e0 tag 0 pio 4096 in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) ata5.00: status: { DRDY } ata5: soft resetting link ata5.00: configured for PIO ata5: EH complete ata5.00: limiting speed to UDMA7:PIO5 ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen ata5.00: cmd 20/00:08:00:00:00/00:00:00:00:00/e0 tag 0 pio 4096 in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) ata5.00: status: { DRDY } ata5: soft resetting link ata5.00: configured for PIO ata5: EH complete ata5.00: limiting speed to PIO0 [...] Thanks, ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] pata_of_platform: fix no irq handling
Tejun Heo wrote: Anton Vorontsov wrote: On Mon, Oct 06, 2008 at 03:41:19PM -0500, Matt Sealey wrote: There is a simple problem with the patch which is that an IRQ 0 can and does actually exist on a bunch of platforms, at least to the best of my knowledge. Checking for -1 (which means for definite, no irq at all, because it is totally unambiguous, as a -1 IRQ numbering is impossible) is more correct. This was discussed years ago. http://lkml.org/lkml/2005/11/22/159 http://lkml.org/lkml/2005/11/22/227 Would this break any existing platforms? If so, can those be fixed together or does it become a much bigger problem that way? Pata_of_platform stacks upon pata_platform. This patch fixes problem concerning definition of no irq without touch any other place. So far I can't see any new problem. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] pata_platform struct resource signness fix
Hi, This patch is to pata_platform.c but at this time, it's powerpc specific because it can only be triggerred using openfirmware, so I post the patch here. The patch is against 2.6.26-rc8. The problem is triggerred when ata device is populated using pata_of_platform.c, and no irq is assigned (poll mode, such as CF card). pata_of_platform_probe() parse device tree and if (ret == NO_IRQ) irq_res.start = irq_res.end = -1; Then irq is 0x, not NULL. Probe will fail coz irq can't be requested. --- (irq_res-start 0) will be true even when it is (-1). When the device has no irq, irq_res-start is assigned (-1). Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/ata/pata_platform.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..b12cd0c 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -123,7 +123,7 @@ int __devinit __pata_platform_probe(struct device *dev, /* * And the IRQ */ - if (irq_res irq_res-start 0) { + if (irq_res irq_res-start != -1) { irq = irq_res-start; irq_flags = irq_res-flags; } -- 1.5.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] pata_platform struct resource signness fix
The alternative fix can be. diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c index 408da30..1f18ad9 100644 --- a/drivers/ata/pata_of_platform.c +++ b/drivers/ata/pata_of_platform.c @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct of_device *ofdev, ret = of_irq_to_resource(dn, 0, irq_res); if (ret == NO_IRQ) - irq_res.start = irq_res.end = -1; + irq_res.start = irq_res.end = 0; else irq_res.flags = 0; I just didn't spend much time to see which is better. Wang Jian wrote: Hi, This patch is to pata_platform.c but at this time, it's powerpc specific because it can only be triggerred using openfirmware, so I post the patch here. The patch is against 2.6.26-rc8. The problem is triggerred when ata device is populated using pata_of_platform.c, and no irq is assigned (poll mode, such as CF card). pata_of_platform_probe() parse device tree and if (ret == NO_IRQ) irq_res.start = irq_res.end = -1; Then irq is 0x, not NULL. Probe will fail coz irq can't be requested. --- (irq_res-start 0) will be true even when it is (-1). When the device has no irq, irq_res-start is assigned (-1). Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/ata/pata_platform.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..b12cd0c 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -123,7 +123,7 @@ int __devinit __pata_platform_probe(struct device *dev, /* * And the IRQ */ - if (irq_res irq_res-start 0) { + if (irq_res irq_res-start != -1) { irq = irq_res-start; irq_flags = irq_res-flags; } ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] pata_platform struct resource signness fix
As I said, I didn't spend much time on the issue, so I don't know which patch is better. (But I do think the patch to pata_of_platform.c is better ATM). AFAIK, powerpc is the only consumer of pata_of_platform.c, so I think this list is better for discussion. I should cc: Anton Vorontsov [EMAIL PROTECTED] but last time I cc: him I got bounced. Leo Li wrote: On Thu, Sep 25, 2008 at 4:36 PM, Wang Jian [EMAIL PROTECTED] wrote: Hi, This patch is to pata_platform.c but at this time, it's powerpc specific because it can only be triggerred using openfirmware, so I post the patch here. The patch is against 2.6.26-rc8. Powerpc isn't the only arch to use openfirmware. Anyway, you have to cc [EMAIL PROTECTED] if you want the patch to be merged. - Leo ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] pata_platform struct resource signness fix
Li Yang wrote: On Thu, Sep 25, 2008 at 4:54 PM, Wang Jian [EMAIL PROTECTED] wrote: The alternative fix can be. This one is better as 0 is defined as 'invalid irq' for all architectures. Added linux-ide and Anton to cc. However, this is not very true. Just git grep #define NO_IRQ and see. It seems that the NO_IRQ is in transition from (-1) to (0). I happened to code the same 2 patches as Anton Vorontsov [EMAIL PROTECTED] had done without knowing his earlier work. I think he was also confused by (-1) and (0). - Leo diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c index 408da30..1f18ad9 100644 --- a/drivers/ata/pata_of_platform.c +++ b/drivers/ata/pata_of_platform.c @@ -52,7 +52,7 @@ static int __devinit pata_of_platform_probe(struct of_device *ofdev, ret = of_irq_to_resource(dn, 0, irq_res); if (ret == NO_IRQ) - irq_res.start = irq_res.end = -1; + irq_res.start = irq_res.end = 0; else irq_res.flags = 0; I just didn't spend much time to see which is better. Wang Jian wrote: Hi, This patch is to pata_platform.c but at this time, it's powerpc specific because it can only be triggerred using openfirmware, so I post the patch here. The patch is against 2.6.26-rc8. The problem is triggerred when ata device is populated using pata_of_platform.c, and no irq is assigned (poll mode, such as CF card). pata_of_platform_probe() parse device tree and if (ret == NO_IRQ) irq_res.start = irq_res.end = -1; Then irq is 0x, not NULL. Probe will fail coz irq can't be requested. --- (irq_res-start 0) will be true even when it is (-1). When the device has no irq, irq_res-start is assigned (-1). Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/ata/pata_platform.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c index 8f65ad6..b12cd0c 100644 --- a/drivers/ata/pata_platform.c +++ b/drivers/ata/pata_platform.c @@ -123,7 +123,7 @@ int __devinit __pata_platform_probe(struct device *dev, /* * And the IRQ */ - if (irq_res irq_res-start 0) { + if (irq_res irq_res-start != -1) { irq = irq_res-start; irq_flags = irq_res-flags; } ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: mpc8541 pci1 ioport allocation address space problem
Confirmed. I had this patch and 2 fixes on dts applied simultaneously, and the board works. I thought this patch was necessary. I reverted this patch and tried again, the board still works. Thanks for your insight. Anyway, the io port read from /proc/ioports is misleading, so I wish it can be changed a little. Regards Benjamin Herrenschmidt wrote: This is expected and should work. Depending on the relative physical addresses of IO space and the order in which the bridges are discovered, the bridge IO ports will look at either positive or negative values. This should be fine, as port numbers are supposed to be 32 bits and in-kernel arithmetic should do the right thing... I suppose unless a driver stores those in a 64 bits integer and doesn't sign extend. I would like to change that whole thing to something more similar to 64 bits where I reserve a portion of the address space for IO ports, though address space on 32 bits platforms is scarce, but nothing I have time to toy with right now. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
mpc8541 pci1 ioport allocation address space problem
Hi, Here I have a 8541 dev board, with 2 e1000 attached to pci0 and a homebrewed addon board inserted into pci1. I am trying to make it work under 2.6.26-rc8 (2.6.26 broken so I am working at rc8). The ioports allocation reads $ cat /proc/ioports -000f : /[EMAIL PROTECTED] 1000-103f : :00:0a.0 1040-107f : :00:0b.0 ffefe000-dfff : /[EMAIL PROTECTED] ffeff000-ffeff00f : 0001:01:0a.0 ffeff010-ffeff01f : 0001:01:0b.0 ffeff020-ffeff02f : 0001:01:0c.0 ffeff030-ffeff03f : 0001:01:0d.0 The port allocation for pci1 looks ridiculous. The addon board doesn't work. After poking around, I find pci_process_bridge_OF_ranges() in arch/powerpc/kernel/pci-common.c --snip-- if (primary) isa_io_base = (unsigned long)hose-io_base_virt; --snip-- then fixup_resource(), _IO_BASE = isa_io_base. fix_resource will use isa_io_base as base address to assign io port address space. This is reasonable, but on my board, pci1's hose-io_base_virt is smaller than pci0's. This lead to [0.064214] PCI::00:0a.0 Resource 4 1000-103f [20101] fixup... [0.064224] fixup_resource() offset=, io_base_virt=fdeb4000, _IO_BASE=fdeb4000, io_base_phys=e200 [0.064232] PCI::00:0a.01000-103f [0.065129] PCI:0001:01:0a.0 Resource 2 1000-100f [20101] fixup... [0.065139] fixup_resource() offset=ffefe000, io_base_virt=fddb2000, _IO_BASE=fdeb4000, io_base_phys=e300 [0.065147] PCI:0001:01:0a.0ffeff000-ffeff00f offset = fddb2000 - fdeb4000 = ffefe000 So far, I think the workaround is to replace the code above with --snip-- if (!isa_io_base) isa_io_base = (unsigned long)hose-io_base_virt; else if ((unsigned long)hose-io_base_virt isa_io_base) isa_io_base = (unsigned long)hose-io_base_virt; --snip-- Then ioport allocation reads (But I haven't test if it works) $ cat /proc/ioports -000f : /[EMAIL PROTECTED] 1000-100f : 0001:01:0a.0 1010-101f : 0001:01:0b.0 1020-102f : 0001:01:0c.0 1030-103f : 0001:01:0d.0 00102000-00201fff : /[EMAIL PROTECTED] 00103000-0010303f : :00:0a.0 00103040-0010307f : :00:0b.0 Can someone gives a better generic fix for this? PS: I attach the kernel log (without workaround) for reference. I added some printk code. [0.064140] PCI: Found :00:0a.0 [8086/1076] 000200 00 [0.064157] pci_read_bases(): IO: (:00:0a.0) l=1001,start=1000,flag= [0.064172] pci :00:0a.0: calling 0xc01e9f98 [0.064182] PCI::00:0a.0 Resource 0 8000-8001 [20204] fixup... [0.064190] PCI::00:0a.08000-8001 [0.064198] PCI::00:0a.0 Resource 2 8002-8002 [20204] fixup... [0.064206] PCI::00:0a.08002-8002 [0.064214] PCI::00:0a.0 Resource 4 1000-103f [20101] fixup... [0.064224] fixup_resource() offset=, io_base_virt=fdeb4000, _IO_BASE=fdeb4000, io_base_phys=e200 [0.064232] PCI::00:0a.01000-103f [0.064241] PCI::00:0a.0 Resource 6 - [27200] is unassigned [0.064268] PCI: Found :00:0b.0 [8086/1076] 000200 00 [0.064284] pci_read_bases(): IO: (:00:0b.0) l=1041,start=1040,flag= [0.064300] pci :00:0b.0: calling 0xc01e9f98 [0.064309] PCI::00:0b.0 Resource 0 8004-8005 [20204] fixup... [0.064316] PCI::00:0b.08004-8005 [0.064325] PCI::00:0b.0 Resource 2 8006-8006 [20204] fixup... [0.064353] PCI::00:0b.08006-8006 [0.064362] PCI::00:0b.0 Resource 4 1040-107f [20101] fixup... [0.064371] fixup_resource() offset=, io_base_virt=fdeb4000, _IO_BASE=fdeb4000, io_base_phys=e200 [0.064379] PCI::00:0b.01040-107f [0.064388] PCI::00:0b.0 Resource 6 - [27200] is unassigned [0.064408] PCI: Fixups for bus :00 [0.064413] PCI: Fixup bus 0 (PHB) [0.064420] Try to map irq for :00:00.0... [0.064447] Try to map irq for :00:0a.0... [0.064459] - got one, spec 2 cells (0x 0x0001...) on /[EMAIL PROTECTED]/[EMAIL PROTECTED] [0.064478] - mapped to linux irq 16 [0.064482] Try to map irq for :00:0b.0... [0.064498] - got one, spec 2 cells (0x0001 0x0001...) on /[EMAIL PROTECTED]/[EMAIL PROTECTED] [0.064510] - mapped to linux irq 17 [0.064517] PCI: Bus scan for :00 returning with max=00 [0.064973] PCI: Scanning bus 0001:01 [0.064989] PCI: Found 0001:01:00.0 [1057/000c] 000b20 00 [
Re: mpc8541 pci1 ioport allocation address space problem
The following patch works fine. diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 063cdd4..4b913e0 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -582,9 +582,19 @@ void __devinit pci_process_bridge_OF_ranges(struct pci_controller *hose, hose-io_base_virt = ioremap(cpu_addr, size); /* Expect trouble if pci_addr is not 0 */ +#if 0 if (primary) isa_io_base = (unsigned long)hose-io_base_virt; +#endif +#if 1 + if (!isa_io_base) + isa_io_base = + (unsigned long)hose-io_base_virt; + else if ((unsigned long)hose-io_base_virt isa_io_base) + isa_io_base = + (unsigned long)hose-io_base_virt; +#endif #endif /* CONFIG_PPC32 */ /* pci_io_size and io_base_phys always represent IO * space starting at 0 so we factor in pci_addr -- On Sat, Sep 20, 2008 at 07:16:25PM +0800, Wang Jian wrote: Hi, Here I have a 8541 dev board, with 2 e1000 attached to pci0 and a homebrewed addon board inserted into pci1. I am trying to make it work under 2.6.26-rc8 (2.6.26 broken so I am working at rc8). The ioports allocation reads $ cat /proc/ioports -000f : /[EMAIL PROTECTED] 1000-103f : :00:0a.0 1040-107f : :00:0b.0 ffefe000-dfff : /[EMAIL PROTECTED] ffeff000-ffeff00f : 0001:01:0a.0 ffeff010-ffeff01f : 0001:01:0b.0 ffeff020-ffeff02f : 0001:01:0c.0 ffeff030-ffeff03f : 0001:01:0d.0 The port allocation for pci1 looks ridiculous. The addon board doesn't work. After poking around, I find pci_process_bridge_OF_ranges() in arch/powerpc/kernel/pci-common.c --snip-- if (primary) isa_io_base = (unsigned long)hose-io_base_virt; --snip-- then fixup_resource(), _IO_BASE = isa_io_base. fix_resource will use isa_io_base as base address to assign io port address space. This is reasonable, but on my board, pci1's hose-io_base_virt is smaller than pci0's. This lead to [0.064214] PCI::00:0a.0 Resource 4 1000-103f [20101] fixup... [0.064224] fixup_resource() offset=, io_base_virt=fdeb4000, _IO_BASE=fdeb4000, io_base_phys=e200 [0.064232] PCI::00:0a.01000-103f [0.065129] PCI:0001:01:0a.0 Resource 2 1000-100f [20101] fixup... [0.065139] fixup_resource() offset=ffefe000, io_base_virt=fddb2000, _IO_BASE=fdeb4000, io_base_phys=e300 [0.065147] PCI:0001:01:0a.0ffeff000-ffeff00f offset = fddb2000 - fdeb4000 = ffefe000 So far, I think the workaround is to replace the code above with --snip-- if (!isa_io_base) isa_io_base = (unsigned long)hose-io_base_virt; else if ((unsigned long)hose-io_base_virt isa_io_base) isa_io_base = (unsigned long)hose-io_base_virt; --snip-- Then ioport allocation reads (But I haven't test if it works) $ cat /proc/ioports -000f : /[EMAIL PROTECTED] 1000-100f : 0001:01:0a.0 1010-101f : 0001:01:0b.0 1020-102f : 0001:01:0c.0 1030-103f : 0001:01:0d.0 00102000-00201fff : /[EMAIL PROTECTED] 00103000-0010303f : :00:0a.0 00103040-0010307f : :00:0b.0 Can someone gives a better generic fix for this? PS: I attach the kernel log (without workaround) for reference. I added some printk code. [0.064140] PCI: Found :00:0a.0 [8086/1076] 000200 00 [0.064157] pci_read_bases(): IO: (:00:0a.0) l=1001,start=1000,flag= [0.064172] pci :00:0a.0: calling 0xc01e9f98 [0.064182] PCI::00:0a.0 Resource 0 8000-8001 [20204] fixup... [0.064190] PCI::00:0a.08000-8001 [0.064198] PCI::00:0a.0 Resource 2 8002-8002 [20204] fixup... [0.064206] PCI::00:0a.08002-8002 [0.064214] PCI::00:0a.0 Resource 4 1000-103f [20101] fixup... [0.064224] fixup_resource() offset=, io_base_virt=fdeb4000, _IO_BASE=fdeb4000, io_base_phys=e200 [0.064232] PCI::00:0a.01000-103f [0.064241] PCI::00:0a.0 Resource 6 - [27200] is unassigned [0.064268] PCI: Found :00:0b.0 [8086/1076] 000200 00 [0.064284] pci_read_bases(): IO: (:00:0b.0) l=1041,start=1040,flag= [0.064300] pci :00:0b.0: calling 0xc01e9f98 [0.064309] PCI::00:0b.0 Resource 0 8004-8005 [20204
Re: [PATCH 0/2] Fix register misuse in drivers/net/phy/marvel.c
Welch, Martyn (GE EntSol, Intelligent Platforms) 写道: Wang Jian wrote: It may not be appropriate to post here instead of netdev, but I am using powerpc dev board and I think there will be someone here who has board to test. Wang Jian 写道: These 2 patches fixed misuse of register for 88e. Notice: These two patches didn't fix some auto selection problem yet. I will discuss the problem in seperate email. Hi, I have a board here with a pair of 88e's, though I'm not sure how to go about testing these patches. It's not the applying or compiling that the problem, but how to test it... Do you have any test case in mind? Hi, The problem I want to fix is: 1. Plug the fiber and start the board in u-boot, the fiber link is established, you can transfer data using fiber link; 2. boot the kernel (not fixed one), when you ifconfig ethX up (ethX uses 88e), the fiber link light goes off, the fiber link is lost; After apply the patch #1, for the step 2, you can ifconfig ethX w.x.y.z and fiber link is ok. You can use the fiber link to transfer data. I my case, the chip is in GMII mode. The patch #2 is obvious but I can't test it myself. BTW, the two patches doesn't fix the problem when you unplug the fiber then plug back. That is another problem I have _partially_ fixed by patch marvell_read_status(). ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/2] Fix copper/fiber auto-selection for 88e1111
The 27.15 bit (MII_M_HWCFG_FIBER_COPPER_AUTO) is disable bit. When set to 1, copper/fiber auto selection is disabled. The current code to enable but actually disable auto selection. Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/net/phy/marvell.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 32a8503..737512c 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -162,7 +162,7 @@ static int m88e_config_init(struct phy_device *phydev) /* Enable Fiber/Copper auto selection */ temp = phy_read(phydev, MII_M_PHY_EXT_SR); - temp |= MII_M_HWCFG_FIBER_COPPER_AUTO; + temp = ~MII_M_HWCFG_FIBER_COPPER_AUTO; phy_write(phydev, MII_M_PHY_EXT_SR, temp); temp = phy_read(phydev, MII_BMCR); -- 1.5.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 0/2] Fix register misuse in drivers/net/phy/marvel.c
These 2 patches fixed misuse of register for 88e. Notice: These two patches didn't fix some auto selection problem yet. I will discuss the problem in seperate email. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 2/2] Fix 88e1111 copper/fiber selection in RGMII mode
MII_M_HWCFG_FIBER_COPPER_RES is a bit of MII_M_PHY_EXT_SR, not MII_M_PHY_EXT_CR. Signed-off-by: Wang Jian [EMAIL PROTECTED] --- drivers/net/phy/marvell.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 737512c..4aa5479 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -158,7 +158,6 @@ static int m88e_config_init(struct phy_device *phydev) { int err; int temp; - int mode; /* Enable Fiber/Copper auto selection */ temp = phy_read(phydev, MII_M_PHY_EXT_SR); @@ -198,9 +197,7 @@ static int m88e_config_init(struct phy_device *phydev) temp = ~(MII_M_HWCFG_MODE_MASK); - mode = phy_read(phydev, MII_M_PHY_EXT_CR); - - if (mode MII_M_HWCFG_FIBER_COPPER_RES) + if (temp MII_M_HWCFG_FIBER_COPPER_RES) temp |= MII_M_HWCFG_MODE_FIBER_RGMII; else temp |= MII_M_HWCFG_MODE_COPPER_RGMII; -- 1.5.5.4 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev