[PATCH v3 2/3] powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
With CONFIG_FSL_IFC now being user-visible, and thus changed from a select to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be selected in defconfigs. Signed-off-by: Esben Haabendal --- arch/powerpc/configs/85xx-hw.config | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config index 524db76f47b7..8aff83217397 100644 --- a/arch/powerpc/configs/85xx-hw.config +++ b/arch/powerpc/configs/85xx-hw.config @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y CONFIG_FSL_CORENET_CF=y CONFIG_FSL_DMA=y CONFIG_FSL_HV_MANAGER=y +CONFIG_FSL_IFC=y CONFIG_FSL_PQ_MDIO=y CONFIG_FSL_RIO=y CONFIG_FSL_XGMAC_MDIO=y @@ -58,6 +59,7 @@ CONFIG_INPUT_FF_MEMLESS=m CONFIG_MARVELL_PHY=y CONFIG_MDIO_BUS_MUX_GPIO=y CONFIG_MDIO_BUS_MUX_MMIOREG=y +CONFIG_MEMORY=y CONFIG_MMC_SDHCI_OF_ESDHC=y CONFIG_MMC_SDHCI_PLTFM=y CONFIG_MMC_SDHCI=y -- 2.45.1
[PATCH v3 3/3] arm64/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
With CONFIG_FSL_IFC now being user-visible, and thus changed from a select to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be selected in defconfig. Signed-off-by: Esben Haabendal --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 2c30d617e180..d101593c3be2 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -1429,6 +1429,7 @@ CONFIG_ARM_MEDIATEK_CCI_DEVFREQ=m CONFIG_EXTCON_PTN5150=m CONFIG_EXTCON_USB_GPIO=y CONFIG_EXTCON_USBC_CROS_EC=y +CONFIG_FSL_IFC=y CONFIG_RENESAS_RPCIF=m CONFIG_IIO=y CONFIG_EXYNOS_ADC=y -- 2.45.1
[PATCH v3 1/3] memory: fsl_ifc: Make FSL_IFC config visible and selectable
While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
Reviewed-by: Miquel Raynal
Signed-off-by: Esben Haabendal
---
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 8efdd1f97139..c82d8d8a16ea 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -167,7 +167,7 @@ config FSL_CORENET_CF
represents a coherency violation.
config FSL_IFC
- bool "Freescale IFC driver" if COMPILE_TEST
+ bool "Freescale IFC driver"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index cbf8ae85e1ae..614257308516 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -234,8 +234,7 @@ config MTD_NAND_FSL_IFC
tristate "Freescale IFC NAND controller"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
- select FSL_IFC
- select MEMORY
+ depends on FSL_IFC
help
Various Freescale chips e.g P1010, include a NAND Flash machine
with built-in hardware ECC capabilities.
--
2.45.1
[PATCH v3 0/3] memory: fsl_ifc: Make FSL_IFC config visible and selectable
While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
Changes in v3:
- Refresh arm64 defconfig.
- Link to v2:
https://lore.kernel.org/r/20240528-fsl-ifc-config-v2-0-5fd7be766...@geanix.com
Changes in v2:
- CONFIG_MTD_NAND_FSL_IFC depends on CONFIG_FSL_IFC instead of select.
- Refresh powerpc config snippet accordingly.
- Link to v1:
https://lore.kernel.org/r/20240523-fsl-ifc-config-v1-1-6eff73bdc...@geanix.com
Signed-off-by: Esben Haabendal
---
Esben Haabendal (3):
memory: fsl_ifc: Make FSL_IFC config visible and selectable
powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
arm64/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
arch/arm64/configs/defconfig| 1 +
arch/powerpc/configs/85xx-hw.config | 2 ++
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig| 3 +--
4 files changed, 5 insertions(+), 3 deletions(-)
---
base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
change-id: 20240523-fsl-ifc-config-c877902b297e
Best regards,
--
Esben Haabendal
Re: [PATCH v2 1/2] memory: fsl_ifc: Make FSL_IFC config visible and selectable
Krzysztof Kozlowski writes:
> On 28/05/2024 15:15, Christophe Leroy wrote:
>> Le 28/05/2024 à 14:28, Esben Haabendal a écrit :
>>> [Vous ne recevez pas souvent de courriers de es...@geanix.com. Découvrez
>>> pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification
>>> ]
>>>
>>> While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
>>> driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
>>> be selectable for platforms using fsl_ifc with NOR flash.
>>
>> I don't understand.
>>
>> Shall I understand :
>>
>> While use of fsl_ifc driver with NAND flash is fine as the fsl_ifc_nand
>> driver selects FSL_IFC automatically,
>>
>> or
>>
>> ..., as the fsl_ifc_nand driver selects FSL_IFC automatically we need
>> the CONFIG_FSL_IFC option to be selectable for platforms using fsl_ifc
>> with NOR flash
>>
>>
>>
>> I'm fine with the fact that you want to be able to select it when you
>> use NOR flashes,
>
> But users are not fine... their memory won't work if they cannot choose
> it (if you meant select=choose).
Exactly. The IFC controller supports both NAND and NOR flashes. Since
commit ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers"),
it has not been possible to use IFC controller for NOR flashes without
selecting the IFC NAND driver (CONFIG_MTD_NAND_FSL_IFC), which is
obviously not how it is supposed to be.
/Esben
Re: [PATCH v2 2/2] powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
Krzysztof Kozlowski writes: > On 28/05/2024 14:28, Esben Haabendal wrote: >> With CONFIG_FSL_IFC now being user-visible, and thus changed from a select >> to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be >> selected in config snippets. >> >> Signed-off-by: Esben Haabendal >> --- >> arch/powerpc/configs/85xx-hw.config | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/powerpc/configs/85xx-hw.config >> b/arch/powerpc/configs/85xx-hw.config >> index 524db76f47b7..8aff83217397 100644 >> --- a/arch/powerpc/configs/85xx-hw.config >> +++ b/arch/powerpc/configs/85xx-hw.config >> @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y >> CONFIG_FSL_CORENET_CF=y >> CONFIG_FSL_DMA=y >> CONFIG_FSL_HV_MANAGER=y >> +CONFIG_FSL_IFC=y > > Does not look like placed according to config order. Correct. > This is not alphabetically sorted, but as Kconfig creates it (make > savedefconfig). Are you sure about this? It looks very much alphabetically sorted, with only two "errors" $ diff -u 85xx-hw.config 85xx-hw.config.sorted --- 85xx-hw.config 2024-05-28 15:05:44.665354428 +0200 +++ 85xx-hw.config.sorted 2024-05-28 15:05:56.102019081 +0200 @@ -15,8 +15,8 @@ CONFIG_DMADEVICES=y CONFIG_E1000E=y CONFIG_E1000=y -CONFIG_EDAC=y CONFIG_EDAC_MPC85XX=y +CONFIG_EDAC=y CONFIG_EEPROM_AT24=y CONFIG_EEPROM_LEGACY=y CONFIG_FB_FSL_DIU=y @@ -71,10 +71,10 @@ CONFIG_MTD_CMDLINE_PARTS=y CONFIG_MTD_NAND_FSL_ELBC=y CONFIG_MTD_NAND_FSL_IFC=y -CONFIG_MTD_RAW_NAND=y CONFIG_MTD_PHYSMAP_OF=y CONFIG_MTD_PHYSMAP=y CONFIG_MTD_PLATRAM=y +CONFIG_MTD_RAW_NAND=y CONFIG_MTD_SPI_NOR=y CONFIG_NETDEVICES=y CONFIG_NVRAM=y I don't think that this file has ever been Kconfig sorted since it was created back in ancient times. And as it is merged with other config snippets using merge_into_defconfig function. I have no idea how to use savedefconfig to maintain such a snippet. It would require doing the reverse of the merge_into_defconfig. >> CONFIG_FSL_PQ_MDIO=y >> CONFIG_FSL_RIO=y > > You also missed to update second defconfig - arm64. Argh. I thought I checked, and it did not need any changes. But it needs to have CONFIG_FSL_IFC=y added. I will add that for v3. /Esben
[PATCH v2 1/2] memory: fsl_ifc: Make FSL_IFC config visible and selectable
While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
Signed-off-by: Esben Haabendal
---
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 8efdd1f97139..c82d8d8a16ea 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -167,7 +167,7 @@ config FSL_CORENET_CF
represents a coherency violation.
config FSL_IFC
- bool "Freescale IFC driver" if COMPILE_TEST
+ bool "Freescale IFC driver"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index cbf8ae85e1ae..614257308516 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -234,8 +234,7 @@ config MTD_NAND_FSL_IFC
tristate "Freescale IFC NAND controller"
depends on FSL_SOC || ARCH_LAYERSCAPE || SOC_LS1021A || COMPILE_TEST
depends on HAS_IOMEM
- select FSL_IFC
- select MEMORY
+ depends on FSL_IFC
help
Various Freescale chips e.g P1010, include a NAND Flash machine
with built-in hardware ECC capabilities.
--
2.45.1
[PATCH v2 2/2] powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
With CONFIG_FSL_IFC now being user-visible, and thus changed from a select to depends in CONFIG_MTD_NAND_FSL_IFC, the dependencies needs to be selected in config snippets. Signed-off-by: Esben Haabendal --- arch/powerpc/configs/85xx-hw.config | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/configs/85xx-hw.config b/arch/powerpc/configs/85xx-hw.config index 524db76f47b7..8aff83217397 100644 --- a/arch/powerpc/configs/85xx-hw.config +++ b/arch/powerpc/configs/85xx-hw.config @@ -24,6 +24,7 @@ CONFIG_FS_ENET=y CONFIG_FSL_CORENET_CF=y CONFIG_FSL_DMA=y CONFIG_FSL_HV_MANAGER=y +CONFIG_FSL_IFC=y CONFIG_FSL_PQ_MDIO=y CONFIG_FSL_RIO=y CONFIG_FSL_XGMAC_MDIO=y @@ -58,6 +59,7 @@ CONFIG_INPUT_FF_MEMLESS=m CONFIG_MARVELL_PHY=y CONFIG_MDIO_BUS_MUX_GPIO=y CONFIG_MDIO_BUS_MUX_MMIOREG=y +CONFIG_MEMORY=y CONFIG_MMC_SDHCI_OF_ESDHC=y CONFIG_MMC_SDHCI_PLTFM=y CONFIG_MMC_SDHCI=y -- 2.45.1
[PATCH v2 0/2] memory: fsl_ifc: Make FSL_IFC config visible and selectable
While use of fsl_ifc driver with NAND flash is fine, as the fsl_ifc_nand
driver selects FSL_IFC automatically, we need the CONFIG_FSL_IFC option to
be selectable for platforms using fsl_ifc with NOR flash.
Fixes: ea0c0ad6b6eb ("memory: Enable compile testing for most of the drivers")
To: Krzysztof Kozlowski
To: Tudor Ambarus
To: Pratyush Yadav
To: Michael Walle
To: Miquel Raynal
To: Richard Weinberger
To: Vignesh Raghavendra
To: Michael Ellerman
To: Nicholas Piggin
To: Christophe Leroy
To: Aneesh Kumar K.V
To: Naveen N. Rao
Cc: linux-ker...@vger.kernel.org
Cc: linux-...@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Esben Haabendal
Changes in v2:
- CONFIG_MTD_NAND_FSL_IFC depends on CONFIG_FSL_IFC instead of select.
- Refresh powerpc config snippet accordingly.
- Link to v1:
https://lore.kernel.org/r/20240523-fsl-ifc-config-v1-1-6eff73bdc...@geanix.com
---
Esben Haabendal (2):
memory: fsl_ifc: Make FSL_IFC config visible and selectable
powerpc/configs: Update defconfig with now user-visible CONFIG_FSL_IFC
arch/powerpc/configs/85xx-hw.config | 2 ++
drivers/memory/Kconfig | 2 +-
drivers/mtd/nand/raw/Kconfig| 3 +--
3 files changed, 4 insertions(+), 3 deletions(-)
---
base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6
change-id: 20240523-fsl-ifc-config-c877902b297e
Best regards,
--
Esben Haabendal
Re: [PATCH 35/41] drivers: tty: serial: 8250: add mapsize to platform data
"Enrico Weigelt, metux IT consult" writes:
> Adding a mapsize field for the 8250 port platform data struct,
> so we can now set the resource size (eg. *1) and don't need
> funny runtime detections like serial8250_port_size() anymore.
>
> For now, serial8250_port_size() is called everytime we need
> the io resource size. That function checks which chip we
> actually have and returns the appropriate size. This approach
> is a bit clumpsy and not entirely easy to understand, and
> it's a violation of layers :p
>
> Obviously, that information cannot change after the driver init,
> so we can safely do that probing once on driver init and just
> use the stored value later.
>
> *1) arch/mips/alchemy/common/platform.c
>
> Signed-off-by: Enrico Weigelt
> ---
> drivers/tty/serial/8250/8250_core.c | 1 +
> include/linux/serial_8250.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c
> b/drivers/tty/serial/8250/8250_core.c
> index e441221..71a398b 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -814,6 +814,7 @@ static int serial8250_probe(struct platform_device *dev)
> uart.port.iotype= p->iotype;
> uart.port.flags = p->flags;
> uart.port.mapbase = p->mapbase;
> + uart.port.mapsize = p->mapsize;
> uart.port.hub6 = p->hub6;
> uart.port.private_data = p->private_data;
> uart.port.type = p->type;
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 5a655ba..8b8183a 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -22,6 +22,7 @@ struct plat_serial8250_port {
> unsigned long iobase; /* io base address */
> void __iomem*membase; /* ioremap cookie or NULL */
> resource_size_t mapbase;/* resource base */
> + resource_size_t mapsize;/* resource size */
> unsigned intirq;/* interrupt number */
> unsigned long irqflags; /* request_irq flags */
> unsigned intuartclk;/* UART clock rate */
Or replace iobase, mapbase and mapsize with a struct resource value?
/Esben
Re: [PATCH 40/41] drivers: tty: serial: helper for setting mmio range
"Enrico Weigelt, metux IT consult" writes: > Introduce a little helpers for settings the mmio range from an > struct resource or start/len parameters with less code. > (also setting iotype to UPIO_MEM) > > Also converting drivers to use these new helpers as well as > fetching mapsize field instead of using hardcoded values. > (the runtime overhead of that should be negligible) > > The idea is moving to a consistent scheme, so later common > calls like request+ioremap combination can be done by generic > helpers. Why not simply replace iobase, mapbase and mapsize with a struct resource value instead? Incidentally, that would allow to specify a memory resource with a parent memory resource :) /Esben
Re: [PATCH 40/41] drivers: tty: serial: helper for setting mmio range
"Enrico Weigelt, metux IT consult" writes:
> @@ -131,7 +133,8 @@ int __init hp300_setup_serial_console(void)
> pr_info("Serial console is HP DCA at select code %d\n", scode);
>
> port.uartclk = HPDCA_BAUD_BASE * 16;
> - port.mapbase = (pa + UART_OFFSET);
> +
> + uart_memres_set_start_len(&port, (pa + UART_OFFSET));
Missing length argument here.
> port.membase = (char *)(port.mapbase + DIO_VIRADDRBASE);
> port.regshift = 1;
> port.irq = DIO_IPL(pa + DIO_VIRADDRBASE);
> diff --git a/drivers/tty/serial/xilinx_uartps.c
> b/drivers/tty/serial/xilinx_uartps.c
> index cf8ca66..895c90c 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1626,8 +1626,7 @@ static int cdns_uart_probe(struct platform_device *pdev)
>* This function also registers this device with the tty layer
>* and triggers invocation of the config_port() entry point.
>*/
> - port->mapbase = res->start;
> - port->mapsize = CDNS_UART_REGISTER_SPACE;
> + uart_memres_set_start_len(res->start, CDNS_UART_REGISTER_SPACE);
Missing 1st (port) argument here.
> port->irq = irq;
> port->dev = &pdev->dev;
> port->uartclk = clk_get_rate(cdns_uart_data->uartclk);
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 5fe2b03..d891c8d 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -427,6 +427,46 @@ void uart_console_write(struct uart_port *port, const
> char *s,
> int uart_match_port(struct uart_port *port1, struct uart_port *port2);
>
> /*
> + * set physical io range from struct resource
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
> + */
> +static inline void uart_memres_set_res(struct uart_port *port,
> +struct resource *res)
> +{
> + if (!res) {
> + port->mapsize = 0;
> + port->mapbase = 0;
> + port->iobase = 0;
> + return;
> + }
> +
> + if (resource_type(res) == IORESOURCE_IO) {
> + port->iotype = UPIO_PORT;
> + port->iobase = resource->start;
> + return;
> + }
> +
> + uart->mapbase = res->start;
> + uart->mapsize = resource_size(res);
> + uart->iotype = UPIO_MEM;
Hardcoding UPIO_MEM like does not work for drivers using other kind of
memory access, like UPIO_MEM16, UPIO_MEM32 and UPIO_MEM32BE.
Why not leave the control of iotype to drivers as before this patch?
> +}
> +
> +/*
> + * set physical io range by start address and length
> + * if resource is NULL, clear the fields
> + * also set the iotype to UPIO_MEM
> + */
> +static inline void uart_memres_set_start_len(struct uart_driver *uart,
> + resource_size_t start,
> + resource_size_t len)
> +{
> + uart->mapbase = start;
> + uart->mapsize = len;
> + uart->iotype = UPIO_MEM;
Same here, other iotype values must be possible.
> +}
> +
> +/*
> * Power Management
> */
> int uart_suspend_port(struct uart_driver *reg, struct uart_port *port);
/Esben
[PATCH 3/3] powerpc: Add .gitignore entry for built DTB files
From: Esben Haabendal Signed-off-by: Esben Haabendal --- arch/powerpc/boot/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore index d61c035..38e6492 100644 --- a/arch/powerpc/boot/.gitignore +++ b/arch/powerpc/boot/.gitignore @@ -29,6 +29,7 @@ zImage.*lds zImage.miboot zImage.pmac zImage.pseries +*.dtb zconf.h zlib.h zutil.h -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] powerpc: Add machine_check cpu function for e300c3 cpus
From: Esben Haabendal
Signed-off-by: Esben Haabendal
---
arch/powerpc/kernel/cputable.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 8084059..f337666 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -1133,6 +1133,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.icache_bsize = 32,
.dcache_bsize = 32,
.cpu_setup = __setup_cpu_603,
+ .machine_check = machine_check_generic,
.num_pmcs = 4,
.oprofile_cpu_type = "ppc/e300",
.oprofile_type = PPC_OPROFILE_FSL_EMB,
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] powerpc: ipic: Fix mcp status helper functions
From: Esben Haabendal
Read and write the SERSR (System Error Status Register) instead of the
SERMR (System Error Mask Register), to actually get and clear the status
bits.
Signed-off-by: Esben Haabendal
---
arch/powerpc/sysdev/ipic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index b287337..2e41a73 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -843,12 +843,12 @@ void ipic_disable_mcp(enum ipic_mcp_irq mcp_irq)
u32 ipic_get_mcp_status(void)
{
- return ipic_read(primary_ipic->regs, IPIC_SERMR);
+ return ipic_read(primary_ipic->regs, IPIC_SERSR);
}
void ipic_clear_mcp_status(u32 mask)
{
- ipic_write(primary_ipic->regs, IPIC_SERMR, mask);
+ ipic_write(primary_ipic->regs, IPIC_SERSR, mask);
}
/* Return an interrupt vector or NO_IRQ if no interrupt is pending. */
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of_mmc_spi: add card detect irq support
On Mon, Aug 30, 2010 at 7:46 PM, Grant Likely wrote: > On Mon, Aug 30, 2010 at 10:04 AM, Esben Haabendal >> Hopefully, arm users will soon enjoy this driver/wrapper soon also. > > My hope is that even on ARM when the device tree is used I'll be able > eliminate IRQs mapped to 0 (but I need to do a lot more research on > how best to do that though). Are you actually using this on ARM? No. > I'm okay with you keeping the NO_IRQ test for the short term though. Ok. I personally don't see a big problem with dropping it either, as I don't actually use it on ARM. But until at least ARM has NO_IRQ==0, I just thought it would be the right thing to try to be compatible. /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] of_mmc_spi: add card detect irq support
Hi
Comments below, and updated patch attached.
On Mon, Aug 30, 2010 at 3:29 PM, Anton Vorontsov wrote:
>>> +static int of_mmc_spi_init(struct device *dev,
>> + irqreturn_t (*irqhandler)(int, void *), void *mmc)
>> +{
>> + struct of_mmc_spi *oms = to_of_mmc_spi(dev);
>
> Please add an empty line here.
Ok.
>> + return request_threaded_irq(
>> + oms->detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc);
>
> I'd write it this way:
>
> return request_threaded_irq(oms->detect_irq, NULL, irqhandler,
>0, dev_name(dev), mmc);
>
> But that's a matter of taste.
Fine with me.
>> +}
>> +
>> +static void of_mmc_spi_exit(struct device *dev, void *mmc)
>> +{
>> + struct of_mmc_spi *oms = to_of_mmc_spi(dev);
>
> Empty line.
Ok.
>> + free_irq(oms->detect_irq, mmc);
>> +}
>> +
>> struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
>> {
>> struct device *dev = &spi->dev;
>> @@ -121,8 +136,14 @@ struct mmc_spi_platform_data
>> *mmc_spi_get_pdata(struct spi_device *spi)
>> if (gpio_is_valid(oms->gpios[WP_GPIO]))
>> oms->pdata.get_ro = of_mmc_spi_get_ro;
>>
>> - /* We don't support interrupts yet, let's poll. */
>> - oms->pdata.caps |= MMC_CAP_NEEDS_POLL;
>> + oms->detect_irq = irq_of_parse_and_map(np, 0);
>> + if (oms->detect_irq != NO_IRQ) {
>
> I'd write "if (oms->detect_irq)", which is a bit more natural
> (and still correct, 0 is the only invalid VIRQ number).
Most other architectures has NO_IRQ defined to -1, so I will stick
with the NO_IRQ comparsion.
Hopefully, arm users will soon enjoy this driver/wrapper soon also.
>> + oms->pdata.init = of_mmc_spi_init;
>> + oms->pdata.exit = of_mmc_spi_exit;
>> + }
>> + else {
>
> } else {
Done.
> Plus, please add an appropriate interrupts = <> bindings into
> Documentation/powerpc/dts-bindings/mmc-spi-slot.txt.
Done.
> And on the next resend, be sure to add Andrew Morton
> , David Brownell
> , and linux-...@vger.kernel.org
> the Cc list.
Should be there now.
/Esben
of_mmc_spi: add card detect irq support
Signed-off-by: Esben Haabendal
---
.../powerpc/dts-bindings/mmc-spi-slot.txt |9 ++-
drivers/mmc/host/of_mmc_spi.c | 26 ++-
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
index c39ac28..89a0084 100644
--- a/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
+++ b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
@@ -7,8 +7,13 @@ Required properties:
- voltage-ranges : two cells are required, first cell specifies minimum
slot voltage (mV), second cell specifies maximum slot voltage (mV).
Several ranges could be specified.
-- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
+
+Optional properties:
+- gpios : may specify GPIOs in this order: Card-Detect GPIO,
Write-Protect GPIO.
+- interrupts : the interrupt of a card detect interrupt.
+- interrupt-parent : the phandle for the interrupt controller that
+ services interrupts for this device.
Example:
@@ -20,4 +25,6 @@ Example:
&qe_pio_d 15 0>;
voltage-ranges = <3300 3300>;
spi-max-frequency = <5000>;
+ interrupts = <42>;
+ interrupt-parent = <&PIC>;
};
diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
index 1247e5d..5530def 100644
--- a/drivers/mmc/host/of_mmc_spi.c
+++ b/drivers/mmc/host/of_mmc_spi.c
@@ -34,6 +34,7 @@ enum {
struct of_mmc_spi {
int gpios[NUM_GPIOS];
bool alow_gpios[NUM_GPIOS];
+ int detect_irq;
struct mmc_spi_platform_data pdata;
};
@@ -61,6 +62,22 @@ static int of_mmc_spi_get_ro(struct device *dev)
return of_mmc_spi_read_gpio(dev, WP_GPIO);
}
+static int of_mmc_spi_init(struct device *dev,
+ irqreturn_t (*irqhandler)(int, void *), void *mmc)
+{
+ struct of_mmc_spi *oms = to_of_mmc_spi(dev);
+
+ return request_threaded_irq(oms->detect_irq, NULL, irqhandler, 0,
+dev_name(dev), mmc);
+}
+
+static void of_mmc_spi_exit(struct device *dev, void *mmc)
+{
+ struct of_mmc_spi *oms = to_of_mmc_spi(dev);
+
+ free_irq(oms->detect_irq, mmc);
+}
+
struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
{
struct device *dev = &spi->dev;
@@ -121,8 +138,13 @@ struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
if (gpio_is_valid(oms->gpios[WP_GPIO]))
oms->pdata.get_ro = of_mmc_spi_get_ro;
- /*
[PATCH] of_mmc_spi: add card detect irq support
Signed-off-by: Esben Haabendal
---
drivers/mmc/host/of_mmc_spi.c | 25 +++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
index 1247e5d..e872b61 100644
--- a/drivers/mmc/host/of_mmc_spi.c
+++ b/drivers/mmc/host/of_mmc_spi.c
@@ -34,6 +34,7 @@ enum {
struct of_mmc_spi {
int gpios[NUM_GPIOS];
bool alow_gpios[NUM_GPIOS];
+ int detect_irq;
struct mmc_spi_platform_data pdata;
};
@@ -61,6 +62,20 @@ static int of_mmc_spi_get_ro(struct device *dev)
return of_mmc_spi_read_gpio(dev, WP_GPIO);
}
+static int of_mmc_spi_init(struct device *dev,
+ irqreturn_t (*irqhandler)(int, void *), void *mmc)
+{
+ struct of_mmc_spi *oms = to_of_mmc_spi(dev);
+ return request_threaded_irq(
+ oms->detect_irq, NULL, irqhandler, 0, dev_name(dev), mmc);
+}
+
+static void of_mmc_spi_exit(struct device *dev, void *mmc)
+{
+ struct of_mmc_spi *oms = to_of_mmc_spi(dev);
+ free_irq(oms->detect_irq, mmc);
+}
+
struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
{
struct device *dev = &spi->dev;
@@ -121,8 +136,14 @@ struct mmc_spi_platform_data
*mmc_spi_get_pdata(struct spi_device *spi)
if (gpio_is_valid(oms->gpios[WP_GPIO]))
oms->pdata.get_ro = of_mmc_spi_get_ro;
- /* We don't support interrupts yet, let's poll. */
- oms->pdata.caps |= MMC_CAP_NEEDS_POLL;
+ oms->detect_irq = irq_of_parse_and_map(np, 0);
+ if (oms->detect_irq != NO_IRQ) {
+ oms->pdata.init = of_mmc_spi_init;
+ oms->pdata.exit = of_mmc_spi_exit;
+ }
+ else {
+ oms->pdata.caps |= MMC_CAP_NEEDS_POLL;
+ }
dev->platform_data = &oms->pdata;
return dev->platform_data;
--
1.7.1.1
--
Esben Haabendal, Senior Software Consultant
DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark
Phone: +45 51 92 53 93, E-mail: e...@doredevelopment.dk
WWW: http://www.doredevelopment.dk
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] gianfar: Fix TX ring processing on SMP machines
On Wed, Mar 3, 2010 at 8:18 PM, Anton Vorontsov
wrote:
> Starting with commit a3bc1f11e9b867a4f49505 ("gianfar: Revive SKB
> recycling") gianfar driver sooner or later stops transmitting any
> packets on SMP machines.
>
> start_xmit() prepares new skb for transmitting, generally it does
> three things:
>
> 1. sets up all BDs (marks them ready to send), except the first one.
> 2. stores skb into tx_queue->tx_skbuff so that clean_tx_ring()
> would cleanup it later.
> 3. sets up the first BD, i.e. marks it ready.
>
> Here is what clean_tx_ring() does:
>
> 1. reads skbs from tx_queue->tx_skbuff
> 2. checks if the *last* BD is ready. If it's still ready [to send]
> then it it isn't transmitted, so clean_tx_ring() returns.
> Otherwise it actually cleanups BDs. All is OK.
>
> Now, if there is just one BD, code flow:
>
> - start_xmit(): stores skb into tx_skbuff. Note that the first BD
> (which is also the last one) isn't marked as ready, yet.
> - clean_tx_ring(): sees that skb is not null, *and* its lstatus
> says that it is NOT ready (like if BD was sent), so it cleans
> it up (bad!)
> - start_xmit(): marks BD as ready [to send], but it's too late.
>
> We can fix this simply by reordering lstatus/tx_skbuff writes.
>
> Reported-by: Martyn Welch
> Bisected-by: Paul Gortmaker
> Signed-off-by: Anton Vorontsov
> Tested-by: Paul Gortmaker
> Tested-by: Martyn Welch
> Cc: Sandeep Gopalpet
> Cc: Stable [2.6.33]
> ---
> drivers/net/gianfar.c | 5 -
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 8bd3c9f..cccb409 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -2021,7 +2021,6 @@ static int gfar_start_xmit(struct sk_buff *skb, struct
> net_device *dev)
> }
>
> /* setup the TxBD length and buffer pointer for the first BD */
> - tx_queue->tx_skbuff[tx_queue->skb_curtx] = skb;
> txbdp_start->bufPtr = dma_map_single(&priv->ofdev->dev, skb->data,
> skb_headlen(skb), DMA_TO_DEVICE);
>
> @@ -2053,6 +2052,10 @@ static int gfar_start_xmit(struct sk_buff *skb, struct
> net_device *dev)
>
> txbdp_start->lstatus = lstatus;
>
> + eieio(); /* force lstatus write before tx_skbuff */
> +
> + tx_queue->tx_skbuff[tx_queue->skb_curtx] = skb;
> +
> /* Update the current skb pointer to the next entry we will use
> * (wrapping if necessary) */
> tx_queue->skb_curtx = (tx_queue->skb_curtx + 1) &
This patch also makes gianfar work stable on mpc8313 with 2.6.33/RT_PREEMPT.
WIthout it, I see exactly the same problems as reported by Anton on SMP.
/Esben
--
Esben Haabendal, Senior Software Consultant
DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark
Phone: +45 51 92 53 93, E-mail: e...@doredevelopment.dk
WWW: http://www.doredevelopment.dk
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
On Tue, 2010-06-08 at 01:18 +0200, Thomas Gleixner wrote: > On Mon, 7 Jun 2010, Esben Haabendal wrote: > > On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner wrote: > > > > > Maybe you understand now, why I was pretty sure upfront, that your > > > approach was wrong even without knowing all the gory details ? :) > > > > I understand. There is a better solution, which is to use threaded > > interrupts where needed. > > > > But I must confess that I am disappointed that you still fail to see > > how the pca953x > > patch actually eliminates the need for serialization. But I don't > > think there is much > > point in going on about that. > > I return the favour of being disappointed that you are still failing > to accept that there is a fundamental difference between "it works in > a particular case" and semantical correctness under all > circumstances. No reason to do that. I must have misunderstood you then. I understand that the general case, I just never got the point where you understood what I did that (I believe) makes it actually work (not just for now, but really work), in my particular case. > There is no place for "it works in a particular case" in a kernel > which runs on a gazillion of different devices unless you want to > create a complete unmaintainable mess. The only way to avoid this is > to be strict about semantics even if it seems unsensible for a > particular case. I won't argue against. Not at all. > Also I explained it to you in great length, that your patch violates > the semantical guarantees of existing functions, but you ignore that > completely. And I answered you several times, that I understand your point, but > It's Open Source, go wild and change it for your particular case - but > don't expect that I accept patches to the generic irq code which will > cause _me_ predictable trouble as I have to deal with the > fallout. Neither expect, that I ack patches to users of that code > which are semantically wrong. > > Just for the record, the pca953x driver is broken vs. the local state > protection anyway as the GPIO functions are not serialized against the > interrupt controller functions. There is no magic which prevents that, > neither on your system nor on any other. The GPIO function should take > buslock when modifying local state and that's one of the reasons why > buslock it is there and cannot go away. Ok, so the genirq buslock should be held in fx. pca953x_gpio_direction_input and pca953x_gpio_direction_output ? That makes sense in combination with the currently implemented bus_sync_unlock. But I still fail to see why it is needed, or even desired, that the setting of direction should be related to the genirq code at all. I see it as a sane seperation to let the actual gpio code handle the direction setting, and the irq code to handle the (in software) irq mask. Driver and/or board code should be sane enough to set pca953x gpio's as input if/when interrupts from them are needed. And they should do this in advance. And by doing it this way, there should not be any hardware state to serialize, and there actually is no pca953x driver local state either, as the pca953x_chip members are either exclusively managed by gpio or irq code. > I'm anticipating that you are going to tell me that it does not matter > on your particular powerpc combined with your usage pattern (i.e no > GPIO users). And I tell you right away, that I'm not interested in > this answer for well explained reasons. No, I don't tie it to my particular usage pattern. I tell you that I don't need it (serialization) on powerpc because my pca953x powerpc patch have removed the need for shared local state between gpio and irq code. Of-course, for the pca953x driver to be acceptable, it would be nice (or more likely, required) to unify it for other archs also, I just don't see how to do that. That said, I will proceed with the request_any_context_irq() in the phy driver instead :-) Case closed for now. /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
On Tue, 2010-06-08 at 08:58 +0200, Thomas Gleixner wrote: > On Mon, 7 Jun 2010, Esben Haabendal wrote: > > > On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner wrote: > > > > > Maybe you understand now, why I was pretty sure upfront, that your > > > approach was wrong even without knowing all the gory details ? :) > > > > I understand. There is a better solution, which is to use threaded > > interrupts where needed. > > FWIW, it just occured to me, that the only reason why the > disable_irq_nosysnc() trips up on the in_atomic() check is your > fiddling with the nested irq dispatcher. > > If you just would have changed the phy driver to > request_irq_any_context() it would have simply worked out of the box, > without any need to remove buslock and changing genirq code at all. > > That would not give you the advantage of getting rid of the two > additional I2C transfers, but that's nn optimzation not a functional > requirement. Now, that is good news. Thanks! /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner wrote: > Maybe you understand now, why I was pretty sure upfront, that your > approach was wrong even without knowing all the gory details ? :) I understand. There is a better solution, which is to use threaded interrupts where needed. But I must confess that I am disappointed that you still fail to see how the pca953x patch actually eliminates the need for serialization. But I don't think there is much point in going on about that. The phy driver should be rewritten to use a threaded handler, it will solve my particular problem. And in the meantime, I have been promised to get the phy interrupts ofloaded to real CPU interrupt lines :-) Oh, I still think that the disable_irq_nosync documentaiton is misleading. Functions that are allowed in a particular context should not call functions that are not allowed to be called in that context. But now I know :-) /Esben -- Esben Haabendal, Senior Software Consultant DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark Phone: +45 51 92 53 93, E-mail: e...@doredevelopment.dk WWW: http://www.doredevelopment.dk ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC][PATCH] irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
On Mon, 2010-06-07 at 00:08 +0200, Thomas Gleixner wrote: > > The only reason for the buslock in the pca9535 driver is to set the > > direction (ie. input) for interrupt pins. On powerpc, I do this in the > > map() > > irq_chip function. So I don't see the need for buslock on powerpc. > > What's so magic about powerpc. Does it magically serialize stuff ? > > The buslock is there for a reason: > > The generic irq code calls irq_chip functions with irq_desc->lock > held and interrupts disabled. So the driver _CANNOT_ access the I2C > bus because that's blocking. Which I don't see a need for. As I mentioned, the only I2C access done at this point is the write to direction register, in case new irq's requires switching a pin from output to input. This can be done on irq_chip->map() on powerpc, without requiring other drivers to know about it. > So the irq_chip functions modify driver local state, which might be > modified by non irq related code as well. Which is not done here. The irq_chip functions modify the following driver local state variables: irq_mask (mask/unmask) irq_trig_fall (set_type) irq_trig_raise (set_type) They are not (to be) modified by other functions. > After releasing desc->lock the genirq code calls the bus unlock > function which does the I2C work and releases the buslock after it's > done. See above. No I2C work is needed. > Therefor we take the buslock before we take desc->lock and release > it after unlocking desc->lock. > > So your removal of buslock removes the protection of the genirq > operation, which is guaranteed to be serialized against other > callers. The driver local state is not longer protected. But I don't see what I need to protect against. No other code is touching driver local state that is also touched by the genirq operations. Or what am I overseeing here? > That's totally unrelated to powerpc and UP, it's simply plain > wrong. And that's why I knew w/o looking at your patch. :) Well, it seems to me that you have a few wrong assumptions about what the genirq operations in the patched pca953x driver is doing, which might warrant a second look at my patch ;-) > You can argue to me in circles, that it is not a problem on your > particular system. I don't care at all. It does not matter as it > violates the semantics and might blow up in other usage scenarios > badly. Hint: think SMP > > And that's why we have request_any_context_irq() in the first place > and want people to realize that the driver which ends up on a I2C irq > controller (which is one of the most braindead ideas hardware folks > came up with ever) needs to be audited and probably modified. I totally agree with the last part. I have asked (and convinced) the hardware people to change that for next revision :-) > > > Just because you have not hit the problem yet does not make it non > > > existant. > > > > Which particular problem should I be on the lookout for in pca953x > > driver? > > See above. Ok, but I believe that these issues have been dealt with by removing the need to do I2C work in the irq_chip functions, and by restricting the driver local state to be only touched by irq_chip functions. Unless I am overlooking something more, I fail to see the need for the buslock in this case (not the general case). > > > Yeah, and at the same time you rip out the buslock. Why are you not > > > sending this patch as well ? Simply because you know that it is > > > utterly wrong and a horrible hack. > > > > No, it was send at the same time, but to the linuxppc-dev. I do not > > You should have CC'ed me as well on that to give me the full context, > so I would have told you in the first reply why it is [pick your > favourite out of my arsenal of swear word combos] but in a more > moderate way as I would have noticed right away that you did not > realize the implications and semantics of buslock and the reasons why > you need to look at the innocent PHY driver. Sorry about that. But your explanations of the buslock in this thread fits very well with my understanding prior to posting here, so I might not be as mis-guided in my attempt. > > see it as utterly wrong, and I hope you will give it a look with an open > > mind, not just assuming that it is shitty crap, utterly wrong or horrible > > hack even before reading it, thanks ;-) > > > > http://patchwork.ozlabs.org/patch/54717/ > > > > It is a longer patch, and I expect that it could be improved quite a > > bit, but I really don't see it as a fundanmentally shitty. > > I do (even before looking at it). Simply because it breaks the > driver. See above. Well, I would be very interested in finding out in more detail where it is broken, as I have replied above to the concerns for I2C work and driver local state race conditions. I am very open about having overlooked something, but I would prefer to know exactly what is wrong. If nothing else, just to be a bit better of next time I enter this area :-) > > >
Re: [PATCH 2/2] gpio: pca953x: add powerpc irq support
On Mon, 2010-06-07 at 01:39 +0200, Thomas Gleixner wrote: > On Fri, 4 Jun 2010, Esben Haabendal wrote: > > NAK for various reasons (no particular order): Ok. I can address 1 and 2, and I believe 3 is still under discussion, at least as far as I am concerned. > 1) That patch misses a sensible changelog. See Documentation/Submit* I will address that. > 2) patch contains several independent changes, which need to be separated I will send a split up patch series. That is, if there is any chance that it will be ACK'ed. No need to waste time on it if it is dead NAK'ed before arriving to list. > > @@ -120,6 +124,10 @@ static int pca953x_gpio_direction_input(struct > > gpio_chip *gc, unsigned off) > > chip = container_of(gc, struct pca953x_chip, gpio_chip); > > > > reg_val = chip->reg_direction | (1u << off); > > + > > + if (reg_val == chip->reg_direction) > > + return 0; > > + > > This is an optimization of its own value. Yes, but is need to avoid doing I2C work from irq_chip map(). > 3) it breaks the driver. See http://lkml.org/lkml/2010/6/6/177 for a >detailed explanation I believe there is still a few things that needs to be discussed before that is closed. > 4) the virq/powerpc churn is horrible and I bet there are sane ways to >solve this, but it leave this to the powerpc experts. Do you suggest that a seperate pca953x driver should be implemented for powerpc? (I guess not). Or do you say that the who irq handling in powerpc should be changed? There must be an acceptable way to extend pca953x.c for the powerpc virq handling and get it accepted in the kernel. /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] powerpc: ipic: use set_irq_chip to ensure irq_chip defaults are applied
On Mon, 2010-06-07 at 01:45 +0200, Thomas Gleixner wrote: > This patch has never been tested with spinlock debugging enabled and > will break SMP as it causes a deadlock on irq_desc->lock. > > Again: See Documentation/Submit* Ok, will do so. But do you see the problem? In __setup_irq(), irq_chip_set_defaults() is called first, and then __irq_set_trigger(), which calls chip->set_type(). When you request an edge irq with ipic, you get the defaults applied to ipic_level_irq_chip, and then it sets ipic_edge_irq_chip, which now is missing startup(), enable(), and so on. Would it be better to change the call order in __setup_irq(), and call irq_chip_set_defaults after __irq_set_trigger() ? Or perhaps even calling it twice (again after __irq_set_trigger()) ? /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] gpio: pca953x: add powerpc irq support
Signed-off-by: Esben Haabendal
---
drivers/gpio/pca953x.c | 347 +---
1 files changed, 240 insertions(+), 107 deletions(-)
diff --git a/drivers/gpio/pca953x.c b/drivers/gpio/pca953x.c
index a2b12aa..a47d55f 100644
--- a/drivers/gpio/pca953x.c
+++ b/drivers/gpio/pca953x.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2005 Ben Gardner
* Copyright (C) 2007 Marvell International Ltd.
+ * Copyright (C) 2010 DoreDevelopment ApS
*
* Derived from drivers/i2c/chips/pca9539.c
*
@@ -67,8 +68,11 @@ struct pca953x_chip {
uint16_t irq_stat;
uint16_t irq_trig_raise;
uint16_t irq_trig_fall;
- int irq_base;
-#endif
+ int irq_base;
+#ifdef CONFIG_PPC
+ struct irq_host *irq_host;
+#endif /* CONFIG_PPC */
+#endif /* CONFIG_GPIO_PCA953X_IRQ */
struct i2c_client *client;
struct pca953x_platform_data *dyn_pdata;
@@ -120,6 +124,10 @@ static int pca953x_gpio_direction_input(struct gpio_chip
*gc, unsigned off)
chip = container_of(gc, struct pca953x_chip, gpio_chip);
reg_val = chip->reg_direction | (1u << off);
+
+ if (reg_val == chip->reg_direction)
+ return 0;
+
ret = pca953x_write_reg(chip, PCA953X_DIRECTION, reg_val);
if (ret)
return ret;
@@ -220,63 +228,41 @@ static void pca953x_setup_gpio(struct pca953x_chip *chip,
int gpios)
}
#ifdef CONFIG_GPIO_PCA953X_IRQ
-static int pca953x_gpio_to_irq(struct gpio_chip *gc, unsigned off)
-{
- struct pca953x_chip *chip;
- chip = container_of(gc, struct pca953x_chip, gpio_chip);
- return chip->irq_base + off;
-}
+#ifdef CONFIG_PPC
-static void pca953x_irq_mask(unsigned int irq)
-{
- struct pca953x_chip *chip = get_irq_chip_data(irq);
-
- chip->irq_mask &= ~(1 << (irq - chip->irq_base));
-}
+#define pca953x_pin_to_virq(chip, pin) \
+ (irq_linear_revmap((chip)->irq_host, pin))
+#define pca953x_virq_to_pin(chip, virq) \
+ (virq_to_hw(virq))
-static void pca953x_irq_unmask(unsigned int irq)
+static void pca953x_irq_mask(unsigned int virq)
{
- struct pca953x_chip *chip = get_irq_chip_data(irq);
-
- chip->irq_mask |= 1 << (irq - chip->irq_base);
+ struct pca953x_chip *chip = get_irq_chip_data(virq);
+ int pin = pca953x_virq_to_pin(chip, virq);
+ pr_debug("%s: virq=%d pin=%d\n", __func__, virq, pin);
+ chip->irq_mask &= ~(1 << pin);
}
-static void pca953x_irq_bus_lock(unsigned int irq)
+static void pca953x_irq_unmask(unsigned int virq)
{
- struct pca953x_chip *chip = get_irq_chip_data(irq);
-
- mutex_lock(&chip->irq_lock);
+ struct pca953x_chip *chip = get_irq_chip_data(virq);
+ int pin = pca953x_virq_to_pin(chip, virq);
+ pr_debug("%s: virq=%d hwirq=%d\n", __func__, virq, pin);
+ chip->irq_mask |= 1 << pin;
}
-static void pca953x_irq_bus_sync_unlock(unsigned int irq)
+static int pca953x_irq_set_type(unsigned int virq, unsigned int type)
{
- struct pca953x_chip *chip = get_irq_chip_data(irq);
- uint16_t new_irqs;
- uint16_t level;
+ struct pca953x_chip *chip = get_irq_chip_data(virq);
+ int pin = pca953x_virq_to_pin(chip, virq);
+ uint16_t mask = 1 << pin;
- /* Look for any newly setup interrupt */
- new_irqs = chip->irq_trig_fall | chip->irq_trig_raise;
- new_irqs &= ~chip->reg_direction;
-
- while (new_irqs) {
- level = __ffs(new_irqs);
- pca953x_gpio_direction_input(&chip->gpio_chip, level);
- new_irqs &= ~(1 << level);
- }
-
- mutex_unlock(&chip->irq_lock);
-}
-
-static int pca953x_irq_set_type(unsigned int irq, unsigned int type)
-{
- struct pca953x_chip *chip = get_irq_chip_data(irq);
- uint16_t level = irq - chip->irq_base;
- uint16_t mask = 1 << level;
+ pr_debug("%s: virq=%d pin=%d type=0x%x\n", __func__, virq, pin, type);
if (!(type & IRQ_TYPE_EDGE_BOTH)) {
- dev_err(&chip->client->dev, "irq %d: unsupported type %d\n",
- irq, type);
+ dev_err(&chip->client->dev, "virq %d: unsupported type %d\n",
+ virq, type);
return -EINVAL;
}
@@ -297,11 +283,140 @@ static struct irq_chip pca953x_irq_chip = {
.name = "pca953x",
.mask = pca953x_irq_mask,
.unmask = pca953x_irq_unmask,
+#ifndef CONFIG_PPC
.bus_lock = pca953x_irq_bus_lock,
.bus_sync_unlock= pca953x_irq_bus_sync_unlock,
+#endif /* !CONFIG_PPC */
.set_type = pca953x_irq_set_type,
};
+static int pca953x_irq_host_match(struct irq_ho
[PATCH 1/2] powerpc: ipic: use set_irq_chip to ensure irq_chip defaults are applied
Signed-off-by: Esben Haabendal
---
arch/powerpc/sysdev/ipic.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index d7b9b9c..8464b86 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -630,10 +630,10 @@ static int ipic_set_irq_type(unsigned int virq, unsigned
int flow_type)
if (flow_type & IRQ_TYPE_LEVEL_LOW) {
desc->status |= IRQ_LEVEL;
desc->handle_irq = handle_level_irq;
- desc->chip = &ipic_level_irq_chip;
+ set_irq_chip(virq, &ipic_level_irq_chip);
} else {
desc->handle_irq = handle_edge_irq;
- desc->chip = &ipic_edge_irq_chip;
+ set_irq_chip(virq, &ipic_edge_irq_chip);
}
/* only EXT IRQ senses are programmable on ipic
--
1.7.1
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/2] ipic and pca953x irq patches
A minor bugfix for the ipic driver, and powerpc support for pca953x irq. Esben Haabendal (2): powerpc: ipic: use set_irq_chip to ensure irq_chip defaults are applied gpio: pca953x: add powerpc irq support arch/powerpc/sysdev/ipic.c |4 +- drivers/gpio/pca953x.c | 347 ++-- 2 files changed, 242 insertions(+), 109 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
Ben Dooks wrote: is there a new version of this patch available? I will catch up on it ASAP. /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
Peter Korsgaard wrote: "Esben" == Esben Haabendal writes: Hi, Esben> It's strange, that line looks perfectly fine when I check the Esben> mail in my GMail inbox and the outbox from the account I sent Esben> it from. Well, it is here and in the archive: http://ozlabs.org/pipermail/linuxppc-dev/2009-May/072274.html If you look at http://www.mail-archive.com/linux-...@vger.kernel.org/msg01050.html instead, the patch is not broken. I find it more likely that the ozlabs.org archive is breaking my lines, than mail-archive.com putting broken lines together again. Please consider using git send-email for patches. I used git imap-send and thunderbird to do it. I will consider git send-email in the future, although I don't think my original e-mail were broken ;-) But ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
> Esben> I've checked both my copy in my "Sent" folder and the copy > Esben> received from the list, and I cannot see any "line break" > Esben> breakage of the patch. > > I guess Wolfram referred to the context line which was clearly word wrapped: > > @@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap, > struct i2c_msg *msgs, int num) > > The other lines look fine. It's strange, that line looks perfectly fine when I check the mail in my GMail inbox and the outbox from the account I sent it from. /Esben -- Esben Haabendal, Senior Software Consultant DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark Phone: +45 51 92 53 93, E-mail: e...@doredevelopment.dk WWW: http://www.doredevelopment.dk ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
On Thu, May 28, 2009 at 9:31 PM, Grant Likely wrote: > On Tue, May 26, 2009 at 5:30 AM, Esben Haabendal wrote: >> On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal wrote: >>> This fixes MAL (arbitration lost) bug caused by illegal use of >>> RSTA (repeated START) after STOP condition generated after last byte >>> of reads. With this patch, it is possible to do an i2c_transfer() with >>> additional i2c_msg's following the I2C_M_RD messages. >>> >>> It still needs to be resolved if it is possible to fix this issue >>> by removing the STOP condition after reads in a robust way. >>> >>> Signed-off-by: Esben Haabendal >>> --- >>> drivers/i2c/busses/i2c-mpc.c | 9 +++-- >>> 1 files changed, 7 insertions(+), 2 deletions(-) >> >> Any blockers to get this accepted? > > It helps if you cc: developers/maintainers of the device. ie. Kumar > for mpc8xxx, me for 52xx. > > This is the first time I noticed your posting. It will take me a few > days before I get a chance to review it. Kumar, will you take a look at this patch? /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
On Thu, May 28, 2009 at 7:17 PM, Wolfram Sang wrote: >> > Any blockers to get this accepted? >> >> It would be nice to get an ack from someone who can actually test >> the driver before getting this merged. > > I wanted to test it, but it does not apply due to line breaks (check > @@-line). Also, I don't really have the time to dig into the topic, so I > would only test it and give a tested-by-tag if it doesn't break anything > here. I think Joakim would be a good candidate for an acked-by . I've checked both my copy in my "Sent" folder and the copy received from the list, and I cannot see any "line break" breakage of the patch. It must be your mail client that is messing with it, so I cannot really do it much better in that way. If necessary, I can push a branch to a public repo you can pull from. /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
On Tue, May 19, 2009 at 7:22 AM, Esben Haabendal wrote: > This fixes MAL (arbitration lost) bug caused by illegal use of > RSTA (repeated START) after STOP condition generated after last byte > of reads. With this patch, it is possible to do an i2c_transfer() with > additional i2c_msg's following the I2C_M_RD messages. > > It still needs to be resolved if it is possible to fix this issue > by removing the STOP condition after reads in a robust way. > > Signed-off-by: Esben Haabendal > --- > drivers/i2c/busses/i2c-mpc.c | 9 +++-- > 1 files changed, 7 insertions(+), 2 deletions(-) Any blockers to get this accepted? /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2] i2c-mpc: generate START condition after STOP caused by read i2c_msg
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads. With this patch, it is possible to do an i2c_transfer() with
additional i2c_msg's following the I2C_M_RD messages.
It still needs to be resolved if it is possible to fix this issue
by removing the STOP condition after reads in a robust way.
Signed-off-by: Esben Haabendal
---
drivers/i2c/busses/i2c-mpc.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 4af5c09..0199f9a 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
}
for (i = 0; ret >= 0 && i < num; i++) {
+ int restart = i;
pmsg = &msgs[i];
dev_dbg(i2c->dev,
"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
pmsg->flags & I2C_M_RD ? "read" : "write",
pmsg->len, pmsg->addr, i + 1, num);
+ if (i > 0 && ((pmsg - 1)->flags & I2C_M_RD))
+ restart = 0;
if (pmsg->flags & I2C_M_RD)
ret =
- mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+restart);
else
ret =
- mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
}
mpc_i2c_stop(i2c);
return (ret < 0) ? ret : num;
--
1.6.3.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
Your new patch also does not work. Have you tested it? I already tried something very much what you sent here, I believe the only difference was that I named the "last" variable "stop". I also tried several other aproaches, and none of them worked. I would appreciate not to have to test all of them seperately again through this mailing list ;-) Anyway, your patch also is in conflict with the MPC8360ERM. The spec specifies that a repeated start must follow an ACK, and not a "NO ACK". When doing a repeated start after a NO ACK, the slave does not ACK the address (I get an RXAK). When doing as specified, ACK'ing the last byte read and then doing a repeated START, i2c_wait() fails due to CSR_MCF bit missing. I thought it would be possible to find somewhere to do a dummy read to get around this, but failed to do so without breaking something else. Could we go forward with my initial patch, and then continue the work on this repeated START approach for future releases? /Esben ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
Hi Your patch (and the small addition to mpc_i2c_start) does not work for me. [ 35.765803] mpc_xfer() [ 35.785480] writeccr 0 [ 35.785505] writeccr 80 [ 35.785523] mpc_xfer: 1 bytes to 2c:W - 1 of 2 messages [ 35.798817] mpc_write addr=2c len=1 restart=0 [ 35.815327] writeccr f0 [ 35.815503] I2C: SR=a2 [ 35.818675] I2C: SR=a6 [ 35.821450] mpc_xfer: 1 bytes to 2c:R - 2 of 2 messages [ 35.827119] mpc_read addr=2c len=1 restart=1 [ 35.837463] writeccr f4 [ 35.837641] I2C: SR=a6 [ 35.840011] writeccr e8 [ 35.840133] I2C: SR=a3 [ 35.843596] writeccr 80 [ 35.843632] mpc_xfer() [ 35.855068] writeccr 0 [ 35.855093] writeccr 80 [ 35.855111] mpc_xfer: 1 bytes to 2c:W - 1 of 2 messages [ 35.865346] mpc_write addr=2c len=1 restart=0 [ 35.870109] writeccr f0 [ 35.870272] I2C: SR=a2 [ 35.873372] I2C: SR=a6 [ 35.875757] mpc_xfer: 1 bytes to 2c:R - 2 of 2 messages [ 35.881606] mpc_read addr=2c len=1 restart=1 [ 35.886290] writeccr f4 [ 35.886463] I2C: SR=a6 [ 35.889425] writeccr e8 [ 35.889575] I2C: SR=a7 [ 35.891944] writeccr 80 [ 35.961177] mpc_xfer() [ 35.972517] writeccr 0 [ 35.972541] writeccr 80 [ 35.972559] mpc_xfer: 1 bytes to 4e:W - 1 of 20 messages [ 35.982628] mpc_write addr=4e len=1 restart=0 [ 35.987389] writeccr f0 [ 35.987424] I2C: SR=b3 [ 35.990386] I2C: MAL [ 35.992971] i2c_wait(address) error: -5 [ 35.997215] writeccr 80 [ 35.997241] Error: i2c_transfer failed: -5 I have now spent a few hours trying a lot of different paths to fix this approach, but I simply cannot find a way to get i2c read to work without a trailing STOP condition with this controller. Is there a problem in applying my patch, as it should be "clean" improvement over the current implementation, which uses STOP condition, but does not work. Until Isomeone finds a way to get it to work without STOP, we will have a working driver, which I assume is what we want. Best regards, Esben Haabendal -- Esben Haabendal, Senior Software Consultant DoréDevelopment ApS, Ved Stranden 1, 9560 Hadsund, DK-Denmark Phone: +45 51 92 53 93, E-mail: e...@doredevelopment.dk WWW: http://www.doredevelopment.dk ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
Wolfram Sang wrote:
On Thu, May 14, 2009 at 10:10:03AM +0200, Esben Haabendal wrote:
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Could you elaborate a bit, please? Like an example when the original bug
occured (so I could reproduce easily) and how this patch actually solves
the problem.
Sure. I used the following code to initialize a CS4265 chip:
(simplified here for the example)
static int
i2c_init_chip(void)
{
int err=0;
char write_dac_control_1[] = { 0x03, chip->regs.dac_control_1 };
char seek_chip_id[] = { 0x01 };
char write_power_control[] = { 0x02, chip->regs.power_control };
struct i2c_msg cs4265_init_cmds[] = {
{ chip->i2c_addr, 0, 2, write_dac_control_1 },
{ chip->i2c_addr, 0, 1, seek_chip_id },
{ chip->i2c_addr, I2C_M_RD, 1, ®s.chip_id },
{ chip->i2c_addr, 0, 2, write_power_control },
};
err = i2c_transfer(i2c, cs4265_init_cmds,
sizeof(cs4265_init_cmds)/sizeof(*cs4265_init_cmds));
if (err < 0) {
printk(KERN_ERR "i2c_transfer failed: %d\n", err);
return err;
}
return 0;
}
I have added some additional debug output and traces something like this:
[ 36.114297] writeccr 80 [MEN]
[ 36.114324] mpc_xfer: 2 bytes to 4f:W - 1 of 5 messages
[ 36.120176] next 1 bytes to 4f:W
[ 36.124159] mpc_write addr=4f len=2
[ 36.128044] writeccr 80 [MEN]
[ 36.128062] writeccr f0 [MEN MIEN MSTA MTX] -> START
[ 36.128219] I2C: SR=a2
[ 36.131528] I2C: SR=a6
[ 36.134406] I2C: SR=a2
[ 36.137165] mpc_xfer: 1 bytes to 4f:W - 2 of 5 messages
[ 36.142802] next 1 bytes to 4f:R
[ 36.146699] mpc_write addr=4f len=1
[ 36.150583] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.150758] I2C: SR=a2
[ 36.153851] I2C: SR=a6
[ 36.156236] mpc_xfer: 1 bytes to 4f:R - 3 of 5 messages
[ 36.162081] next 2 bytes to 4f:W
[ 36.166062] mpc_read addr=4f len=1
[ 36.169858] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.170031] I2C: SR=a6
[ 36.172993] writeccr e8 [MEN MIEN MSTA TXAK] -> receive & no ACK
[ 36.173143] I2C: SR=a7
[ 36.175511] writeccr c8 [MEN MIEN TXAK] -> STOP
[ 36.175529] mpc_xfer: 2 bytes to 4f:W - 4 of 5 messages
[ 36.181804] next 2 bytes to 4f:W
[ 36.185788] mpc_write addr=4f len=2
[ 36.189672] writeccr f4 [MEN MIEN MSTA MTX RSTA] -> repeated START
[ 36.189704] I2C: SR=b7
[ 36.192072] I2C: MAL
[ 36.195075] i2c_wait(address) error: -5
[ 36.199311] writeccr 80
The problem is that after the STOP condition, the following i2c_msg will be
attempted with a repeated START, which according to specification will
cause a MAL, which also happens. My MPC8360ERM reads:
"Attempting a repeated START at the wrong time (or if the bus is owned
by another master), results in loss of arbitration."
Which I know read as it that we must own the I2C bus when using RSTA.
Signed-off-by: Esben Haabendal
---
drivers/i2c/busses/i2c-mpc.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 4af5c09..9fe05d9 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
}
for (i = 0; ret >= 0 && i < num; i++) {
+ int restart = i;
pmsg = &msgs[i];
dev_dbg(i2c->dev,
"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
pmsg->flags & I2C_M_RD ? "read" : "write",
pmsg->len, pmsg->addr, i + 1, num);
+ if (i > 0 && ((pmsg-1)->flags & I2C_M_RD))
CodingStyle: Spaces around '-'
Ok.
+ restart = 0;
if (pmsg->flags & I2C_M_RD)
ret =
- mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+restart);
else
ret =
- mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
}
mpc_i2c_stop(i2c);
return (ret < 0) ? ret : num;
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] i2c-mpc: generate START condition after STOP caused by read i2c_msg
This fixes MAL (arbitration lost) bug caused by illegal use of
RSTA (repeated START) after STOP condition generated after last byte
of reads.
Signed-off-by: Esben Haabendal
---
drivers/i2c/busses/i2c-mpc.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 4af5c09..9fe05d9 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -456,17 +456,22 @@ static int mpc_xfer(struct i2c_adapter *adap,
struct i2c_msg *msgs, int num)
}
for (i = 0; ret >= 0 && i < num; i++) {
+ int restart = i;
pmsg = &msgs[i];
dev_dbg(i2c->dev,
"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
pmsg->flags & I2C_M_RD ? "read" : "write",
pmsg->len, pmsg->addr, i + 1, num);
+ if (i > 0 && ((pmsg-1)->flags & I2C_M_RD))
+ restart = 0;
if (pmsg->flags & I2C_M_RD)
ret =
- mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+restart);
else
ret =
- mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+ mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len,
+ restart);
}
mpc_i2c_stop(i2c);
return (ret < 0) ? ret : num;
--
1.6.0.4
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] CPM2 i2c driver
Hi
Here is an i2c driver for Freescale processors with CPM2. It is based on
previous work by Angelos Manousaridis which again were based on a driver
from Heiko Schocher.
I am using it on an in-house MPC8270 target, and don't have access to
any other boards with CPM2. So if anybody is able to test this driver
with a supported board, I would by happy to help out. It would be nice
to have it tested and integrated with one or more reference boards, and
have the needed dts changed included also.
Best regards,
Esben Haabendal
Original Message
Subject: i2c-cpm2.patch
Date: Fri, 20 Jun 2008 11:02:3ga3 +0200
From: Esben Haabendal <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED]
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index ca54563..662c076 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -537,6 +537,45 @@ static int __init fsl_i2c_of_init(void)
i++;
}
+ for_each_compatible_node(np, NULL, "fsl-cpm2-i2c") {
+ struct resource r[2];
+ struct fsl_i2c_platform_data i2c_data;
+ const unsigned char *flags = NULL;
+
+ printk(KERN_INFO "%s: fsl-cpm2-i2c node found\n", __FUNCTION__);
+
+ memset(&r, 0, sizeof(r));
+ memset(&i2c_data, 0, sizeof(i2c_data));
+
+ ret = of_address_to_resource(np, 0, &r[0]);
+ if (ret)
+ goto err;
+ printk(KERN_INFO "%s: r[0] = %x %x\n",
+ __FUNCTION__, r[0].start, r[0].end);
+
+ of_irq_to_resource(np, 0, &r[1]);
+ printk(KERN_INFO "%s: r[1] = %x %x\n",
+ __FUNCTION__, r[1].start, r[2].end);
+
+ i2c_dev = platform_device_register_simple(
+ "fsl-cpm2-i2c", i, r, 2);
+ if (IS_ERR(i2c_dev)) {
+ ret = PTR_ERR(i2c_dev);
+ goto err;
+ }
+
+ i2c_data.device_flags = 0;
+
+ ret =
+ platform_device_add_data(i2c_dev, &i2c_data,
+sizeof(struct
+ fsl_i2c_platform_data));
+ if (ret)
+ goto unreg;
+
+ of_register_i2c_devices(np, i++);
+ }
+
return 0;
unreg:
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 48438cc..3715d12 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -305,6 +305,16 @@ config I2C_MPC
This driver can also be built as a module. If so, the module
will be called i2c-mpc.
+config I2C_CPM2
+ tristate "MPC CPM2"
+ depends on I2C && PPC32 && CPM2
+ help
+ If you say yes to this option, support will be included for the
+ I2C interface on PPC with CPM2
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-cpm2.
+
config I2C_NFORCE2
tristate "Nvidia nForce2, nForce3 and nForce4"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index e8c882a..544decb 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_I2C_VIAPRO) += i2c-viapro.o
obj-$(CONFIG_I2C_VOODOO3) += i2c-voodoo3.o
obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
obj-$(CONFIG_SCx200_I2C) += scx200_i2c.o
+obj-$(CONFIG_I2C_CPM2) += i2c-cpm2.o
ifeq ($(CONFIG_I2C_DEBUG_BUS),y)
EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/i2c/busses/i2c-cpm2.c b/drivers/i2c/busses/i2c-cpm2.c
new file mode 100644
index 000..cca1eba
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cpm2.c
@@ -0,0 +1,609 @@
+/*
+ * (C) Copyright 2005
+ * Heiko Schocher <[EMAIL PROTECTED]>
+ *
+ * (C) Copyright 2006
+ * Angelos Manousaridis <[EMAIL PROTECTED]>
+ *
+ * (C) Copyright 2008
+ * Esben Haabendal <[EMAIL PROTECTED]>
+ *
+ * This is a combined i2c adapter and algorithm driver for
+ * PPC with CPM2, based on initial work by first Heiko Schocher,
+ * reworked by Angelos Manousaridis, and finally reshaped a bit by
+ * Esben Haabendal.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CPM_MAX_READ 513
+
+struct cpm2_i2c {
+ struct i2c_adapter adap;
+ wait_queue_head_t queue;
+ u32 irq;
+ spinlock_t lock;
+ i2c_cpm2_t *regs;
+ u16 *base;
+ ushort dp_offset;
+ iic_t *dp_addr;
+ ushort rbdf_offset
[PATCH] cpm: rounding of brg clockdivider
Do rounding of brg clockdivider instead of truncate to get more precise baudrates Something similar might be needed for cpm_fastbrg... --- commit 52d631eb8f64cef794d6aa66494e253cf268894e tree 956149a0eb5beb9afb280f4593615929eab7b779 parent 300070dd6b5e71af0c6fbecd32388905dbdd3ea5 author Esben Haabendal <[EMAIL PROTECTED]> Wed, 19 Sep 2007 13:18:59 +0200 committer Esben Haabendal <[EMAIL PROTECTED]> Wed, 19 Sep 2007 13:18:59 +0200 arch/powerpc/sysdev/cpm2_common.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/sysdev/cpm2_common.c b/arch/powerpc/sysdev/cpm2_common.c index 9058da2..a2c8157 100644 --- a/arch/powerpc/sysdev/cpm2_common.c +++ b/arch/powerpc/sysdev/cpm2_common.c @@ -102,7 +102,9 @@ cpm_setbrg(uint brg, uint rate) brg -= 4; } bp += brg; - out_be32(bp, (((BRG_UART_CLK / rate) - 1) << 1) | CPM_BRG_EN); + /* Set the BRG clock divider to get the best match to the requested +* baudrate (rounding required) */ + out_be32(bp, ((BRG_UART_CLK*2)/rate)+1)/2)-1) << 1) | CPM_BRG_EN); cpm2_unmap(bp); } !-----flip- -- Esben Haabendal Embedded Software Consultant Doré Development ApS ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH 6/9] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
Hi Scott, A minor error handling bug > + const u32 *data = of_get_property(np, "phy-handle", &len); > + if (!data || len != 4) > + return -EINVAL; > + > + phynode = of_find_node_by_phandle(*data); > + if (!phynode) > + return -EINVAL; > + > + mdionode = of_get_parent(phynode); > + if (!phynode) if (!mdionode) > + goto out_put_phy; Best regards, Esben ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Where did the fs_enet patches go?
On Wed, 19 Sep 2007 11:40:35 +0400, Vitaly Bordug <[EMAIL PROTECTED]> wrote: > On Wed, 19 Sep 2007 09:05:16 +0200 >> Where did the patches containing the reworked fs_enet drivers and the >> generic bitbanged MDIO library go? >> I am talking about these 8 patches: >> http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg03184.html >> >> They seem to have been removed from >> http://patchwork.ozlabs.org/linuxppc/ ... >> >> AFAICS, they have not been merged in neither galak or paulus git. >> >> It works nicely here at my mpc8270 target btw. :-) >> > iirc they are slowly spinning with netdev/Jeff Garzik... Scott would know > for sure :) Ah, I see. Thanks. Most of it is at least ack'ed by Jeff. But he did not seem to like the generic bitbanged MDIO library. Scott, what is the plan with that part? I need it :-) /Esben -- Esben Haabendal Embedded Software Consultant Doré Development ApS ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Where did the fs_enet patches go?
Hi Where did the patches containing the reworked fs_enet drivers and the generic bitbanged MDIO library go? I am talking about these 8 patches: http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg03184.html They seem to have been removed from http://patchwork.ozlabs.org/linuxppc/ ... AFAICS, they have not been merged in neither galak or paulus git. It works nicely here at my mpc8270 target btw. :-) -- Esben Haabendal Embedded Software Consultant Doré Development ApS ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Problems with mii-bitbang.c on MPC8270
e+0x78/0x80 [3.193512] LR [c014cf18] device_release+0x78/0x80 [3.198299] Call Trace: [3.200738] [c3fe1e50] [c014cf18] device_release+0x78/0x80 (unreliable) [3.207353] [c3fe1e60] [c012dfc8] kobject_cleanup+0x70/0xac [3.212923] [c3fe1e80] [c012f0dc] kref_put+0x54/0x6c [3.217884] [c3fe1e90] [c012df48] kobject_put+0x24/0x34 [3.223107] [c3fe1ea0] [c014d0f4] put_device+0x1c/0x2c [3.228242] [c3fe1eb0] [c0158640] mdiobus_unregister+0x2c/0x58 [3.234074] [c3fe1ec0] [c015b51c] fs_enet_mdio_remove+0x24/0x6c [3.239992] [c3fe1ee0] [c0167754] of_platform_device_remove+0x30/0x44 [3.246433] [c3fe1ef0] [c0150698] __device_release_driver+0x84/0xc0 [3.252700] [c3fe1f00] [c0150e98] driver_detach+0x144/0x180 [3.258271] [c3fe1f20] [c014f770] bus_remove_driver+0xa4/0xd8 [3.264016] [c3fe1f40] [c0150f24] driver_unregister+0x10/0x20 [3.269760] [c3fe1f50] [c0009c10] of_unregister_platform_driver+0x14/0x24 [3.276549] [c3fe1f60] [c015b4e4] fs_enet_mdio_bb_exit+0x24/0x38 [3.282555] [c3fe1f70] [c02431c8] kernel_init+0xac/0x2a0 [3.287864] [c3fe1ff0] [c0010028] kernel_thread+0x44/0x60 [3.293260] Instruction dump: [3.296220] 2f80 409effd0 8124011c 2f89 419e0010 800900e0 2f80 409effb8 [3.303966] 3c60c023 38633654 388400a4 4becf649 <0fe0> 4bac 9421ffe0 7c0802a6 [3.313784] my flash device: 0x200 at 0xfe00 [3.320283] my onboard flash: Found 2 x16 devices at 0x0 in 32-bit bank [3.326403] Amd/Fujitsu Extended Query Table at 0x0040 [3.331603] Using buffer write method [3.335244] my onboard flash: CFI does not contain boot bank location. Assuming top. [3.343966] number of CFI chips: 1 [3.347341] cfi_cmdset_0002: Disabling erase-suspend-program due to code brokenness. [3.355107] Creating 2 MTD partitions on "my onboard flash": [3.361719] 0x-0x0008 : "U-Boot" [3.365972] mtd: Giving out device 0 to U-Boot [3.371647] 0x0008-0x0200 : "JFFS2" [3.374621] mtd: Giving out device 1 to JFFS2 [ 3.380226] my flash device initialized [3.383764] TCP cubic registered [3.387023] NET: Registered protocol family 1 [3.391316] NET: Registered protocol family 17 [3.897456] f0010d40:00 not found [3.899235] eth0: Could not attach to PHY [3.903224] IP-Config: Failed to open eth0 [3.907315] IP-Config: Device `eth0' not found. [3.911877] driver_probe_done: probe_count = 0 [3.916681] Looking up port of RPC 13/2 on 192.168.18.103 -- Esben Haabendal Embedded Software Consultant Doré Development ApS ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Configuring for UPM on MPC82xx using new device binding from Scott
Hi Is it possible to configure chipselect to use UPM A and UPM B in an MPC8270 chip using the new CPM device bindings patches from Scott? I need to pass in the 3 bits for MS (machine select to the relevant MRx register). Best regards, Esben Haabendal ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
