mbox series

[RESEND,v7,00/37] Device Tree support for SH7751 based board

Message ID cover.1712205900.git.ysato@users.sourceforge.jp
Headers show
Series Device Tree support for SH7751 based board | expand

Message

Yoshinori Sato April 4, 2024, 5:14 a.m. UTC
Sorry. previus mail is thread broken.

This is an updated version of something I wrote about 7 years ago.
Minimum support for R2D-plus and LANDISK.
I think R2D-1 will work if you add AX88796 to dts.
And board-specific functions and SCI's SPI functions are not supported.

You can get it working with qemu found here.
https://gitlab.com/yoshinori.sato/qemu/-/tree/landisk

v7 changes.
- sh/kernel/setup.c: fix kernel parameter handling.
- clk-sh7750.c: cleanup.
- sh_tmu.c: cleanup.
- irq-renesas-sh7751.c: IPR definition move to code.
- irq-renesas-sh7751irl.c: update register definition.
- pci-sh7751.c: Register initialization fix. 
- sm501 and sm501fb: Re-design Device Tree properties.

v6 changes.
- pci-sh7751: merge register define.
- pci-sh7751: use 'dma-ranges' property.
- pci-sh7751: rename general PCI properties.
- sm501 and sm501fb: Re-design Device Tree properties.
- sh/kernel/setup: cleanup command line setup.
- irq-sh7751.c: some cleanup.

v5 changes.
- pci-sh7751: revert header changes. and some fix in previuous driver.
- sh/kernel/iomap.c: Use SH io functions.
- sm501 and sm501fb: re-write DT support.

v4 changes.
- cpg-sh7750: use clk-divider and clk-gate.
- pci-sh7751: unified header files to old PCI driver.
- irq-renesas-sh7751: IPR registers direct mapping.
- irq-renesas-sh7751irl: useful register bit mapping.
- sm501 and sm501fb: re-write dt parser.
- j2_minus: fix build error.
- dt-binding schema: fix some errors.
- *.dts: cleanup.

v3 changes.
- Rewrite clk drivers.
- Added sh_tmu to OF support.
- Cleanup PCI stuff.
- Update sm501 and sm501fb OF support.
- Update devicetree and documents.

v2 changes.
- Rebasing v6,6-rc1
- re-write irqchip driver.
- Add binding documents.
- Cleanup review comment.

Yoshinori Sato (37):
  sh: passing FDT address to kernel startup.
  sh: Kconfig unified OF supported targets.
  sh: Enable OF support for build and configuration.
  dt-bindings: interrupt-controller: Add header for Renesas SH3/4 INTC.
  sh: GENERIC_IRQ_CHIP support for CONFIG_OF=y
  sh: kernel/setup Update DT support.
  sh: Fix COMMON_CLK support in CONFIG_OF=y.
  clocksource: sh_tmu: CLOCKSOURCE support.
  dt-binding: Add compatible SH7750 SoC
  sh: Common PCI Framework driver support.
  pci: pci-sh7751: Add SH7751 PCI driver
  dt-bindings: pci: pci-sh7751: Add SH7751 PCI
  dt-bindings: clock: sh7750-cpg: Add renesas,sh7750-cpg header.
  clk: Compatible with narrow registers
  clk: renesas: Add SH7750/7751 CPG Driver
  irqchip: Add SH7751 INTC driver
  dt-bindings: interrupt-controller: renesas,sh7751-intc: Add
    json-schema
  irqchip: SH7751 external interrupt encoder with enable gate.
  dt-bindings: interrupt-controller: renesas,sh7751-irl-ext: Add
    json-schema
  serial: sh-sci: fix SH4 OF support.
  dt-bindings: serial: renesas,scif: Add scif-sh7751.
  dt-bindings: display: smi,sm501: SMI SM501 binding json-schema
  dt-bindings: display: sm501 register definition helper
  mfd: sm501: Convert platform_data to OF property
  dt-binding: sh: cpus: Add SH CPUs json-schema
  dt-bindings: vendor-prefixes: Add iodata
  dt-bindings: ata: ata-generic: Add new targets
  dt-bindings: soc: renesas: sh: Add SH7751 based target
  sh: SH7751R SoC Internal peripheral definition dtsi.
  sh: add RTS7751R2D Plus DTS
  sh: Add IO DATA LANDISK dts
  sh: Add IO DATA USL-5P dts
  sh: j2_mimas_v2.dts update
  sh: Add dtbs target support.
  sh: RTS7751R2D Plus OF defconfig
  sh: LANDISK OF defconfig
  sh: j2_defconfig: update

 .../devicetree/bindings/ata/ata-generic.yaml  |   2 +
 .../bindings/clock/renesas,sh7750-cpg.yaml    | 105 ++++
 .../bindings/display/smi,sm501.yaml           | 398 +++++++++++++++
 .../renesas,sh7751-intc.yaml                  |  53 ++
 .../renesas,sh7751-irl-ext.yaml               |  57 +++
 .../bindings/pci/renesas,sh7751-pci.yaml      |  89 ++++
 .../bindings/serial/renesas,scif.yaml         |   1 +
 .../devicetree/bindings/sh/cpus.yaml          |  63 +++
 .../devicetree/bindings/soc/renesas/sh.yaml   |  27 +
 .../bindings/timer/renesas,tmu.yaml           |   2 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/sh/Kconfig                               |  33 +-
 arch/sh/boards/Kconfig                        |  23 +-
 arch/sh/boards/of-generic.c                   |  28 +-
 arch/sh/boot/compressed/head_32.S             |   5 +-
 arch/sh/boot/dts/Makefile                     |   5 +
 arch/sh/boot/dts/j2_mimas_v2.dts              |   2 +-
 arch/sh/boot/dts/landisk.dts                  |  77 +++
 arch/sh/boot/dts/rts7751r2dplus.dts           | 169 ++++++
 arch/sh/boot/dts/sh7751r.dtsi                 | 105 ++++
 arch/sh/boot/dts/usl-5p.dts                   |  85 ++++
 arch/sh/configs/j2_defconfig                  |  11 +-
 arch/sh/configs/landisk-of_defconfig          | 104 ++++
 arch/sh/configs/rts7751r2dplus-of_defconfig   |  75 +++
 arch/sh/drivers/Makefile                      |   2 +
 arch/sh/include/asm/io.h                      |   8 +
 arch/sh/include/asm/irq.h                     |  10 +-
 arch/sh/include/asm/pci.h                     |   4 +
 arch/sh/include/asm/setup.h                   |   1 +
 arch/sh/kernel/cpu/Makefile                   |   6 +-
 arch/sh/kernel/cpu/irq/imask.c                |  17 +
 arch/sh/kernel/cpu/sh4/Makefile               |   3 +
 arch/sh/kernel/iomap.c                        |  18 +
 arch/sh/kernel/setup.c                        |  36 +-
 arch/sh/kernel/time.c                         |  12 +
 drivers/clk/clk-divider.c                     |  56 +-
 drivers/clk/clk-gate.c                        |  62 ++-
 drivers/clk/renesas/Kconfig                   |  13 +-
 drivers/clk/renesas/Makefile                  |   1 +
 drivers/clk/renesas/clk-sh7750.c              | 480 ++++++++++++++++++
 drivers/clocksource/sh_tmu.c                  | 198 +++++---
 drivers/irqchip/Kconfig                       |  15 +
 drivers/irqchip/Makefile                      |   3 +
 drivers/irqchip/irq-renesas-sh7751.c          | 282 ++++++++++
 drivers/irqchip/irq-renesas-sh7751irl.c       | 221 ++++++++
 drivers/mfd/sm501.c                           | 315 ++++++++++++
 drivers/pci/controller/Kconfig                |   9 +
 drivers/pci/controller/Makefile               |   1 +
 drivers/pci/controller/pci-sh7751.c           | 342 +++++++++++++
 drivers/tty/serial/Kconfig                    |   2 +-
 drivers/tty/serial/sh-sci.c                   |   6 +-
 drivers/video/fbdev/sm501fb.c                 | 106 ++++
 include/dt-bindings/clock/sh7750-cpg.h        |  26 +
 include/dt-bindings/display/sm501.h           |  76 +++
 .../renesas,sh7751-intc.h                     |  19 +
 include/linux/clk-provider.h                  |  22 +-
 include/linux/sh_intc.h                       |   7 +-
 57 files changed, 3713 insertions(+), 187 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/renesas,sh7750-cpg.yaml
 create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-intc.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
 create mode 100644 Documentation/devicetree/bindings/sh/cpus.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/renesas/sh.yaml
 create mode 100644 arch/sh/boot/dts/landisk.dts
 create mode 100644 arch/sh/boot/dts/rts7751r2dplus.dts
 create mode 100644 arch/sh/boot/dts/sh7751r.dtsi
 create mode 100644 arch/sh/boot/dts/usl-5p.dts
 create mode 100644 arch/sh/configs/landisk-of_defconfig
 create mode 100644 arch/sh/configs/rts7751r2dplus-of_defconfig
 create mode 100644 drivers/clk/renesas/clk-sh7750.c
 create mode 100644 drivers/irqchip/irq-renesas-sh7751.c
 create mode 100644 drivers/irqchip/irq-renesas-sh7751irl.c
 create mode 100644 drivers/pci/controller/pci-sh7751.c
 create mode 100644 include/dt-bindings/clock/sh7750-cpg.h
 create mode 100644 include/dt-bindings/display/sm501.h
 create mode 100644 include/dt-bindings/interrupt-controller/renesas,sh7751-intc.h

Comments

Krzysztof Kozlowski April 4, 2024, 6:07 a.m. UTC | #1
On 04/04/2024 06:59, Yoshinori Sato wrote:
> Renesas SH7751 Interrupt controller priority register define.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Nothing improved, still NAK.

This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof
Bjorn Helgaas April 4, 2024, 1:46 p.m. UTC | #2
On Thu, Apr 04, 2024 at 01:59:25PM +0900, Yoshinori Sato wrote:
> This is an updated version of something I wrote about 7 years ago.
> Minimum support for R2D-plus and LANDISK.
> I think R2D-1 will work if you add AX88796 to dts.
> And board-specific functions and SCI's SPI functions are not supported.

My comments/questions from
https://lore.kernel.org/r/20231120181600.GA205977@bhelgaas
https://lore.kernel.org/r/20231016172742.GA1215127@bhelgaas
still apply.
Mayank Rana April 8, 2024, 9:32 p.m. UTC | #3
Hi,

On 4/3/2024 9:59 PM, Yoshinori Sato wrote:
> Renesas SH7751 CPU Internal PCI Controller driver.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>   drivers/pci/controller/Kconfig      |   9 +
>   drivers/pci/controller/Makefile     |   1 +
>   drivers/pci/controller/pci-sh7751.c | 342 ++++++++++++++++++++++++++++
>   3 files changed, 352 insertions(+)
>   create mode 100644 drivers/pci/controller/pci-sh7751.c
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index e534c02ee34f..a2fd917a2e03 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -353,6 +353,15 @@ config PCIE_XILINX_CPM
>   	  Say 'Y' here if you want kernel support for the
>   	  Xilinx Versal CPM host bridge.
>   
> +config PCI_SH7751
> +	bool "Renesas SH7751 PCI controller"
> +	depends on OF
> +	depends on CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7751R || COMPILE_TEST
> +	select PCI_HOST_COMMON
> +	help
> +	  Say 'Y' here if you want kernel to support the Renesas SH7751 PCI
> +	  Host Bridge driver.
> +
>   source "drivers/pci/controller/cadence/Kconfig"
>   source "drivers/pci/controller/dwc/Kconfig"
>   source "drivers/pci/controller/mobiveil/Kconfig"
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index f2b19e6174af..aa97e5d74e58 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCI_LOONGSON) += pci-loongson.o
>   obj-$(CONFIG_PCIE_HISI_ERR) += pcie-hisi-error.o
>   obj-$(CONFIG_PCIE_APPLE) += pcie-apple.o
>   obj-$(CONFIG_PCIE_MT7621) += pcie-mt7621.o
> +obj-$(CONFIG_PCI_SH7751) += pci-sh7751.o
>   
>   # pcie-hisi.o quirks are needed even without CONFIG_PCIE_DW
>   obj-y				+= dwc/
> diff --git a/drivers/pci/controller/pci-sh7751.c b/drivers/pci/controller/pci-sh7751.c
> new file mode 100644
> index 000000000000..a5340689f737
> --- /dev/null
> +++ b/drivers/pci/controller/pci-sh7751.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SH7751 PCI driver
> + * Copyright (C) 2023 Yoshinori Sato
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci-ecam.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/dma-direct.h>
> +#include <asm/addrspace.h>
can you consider to rearranging headers into alphabetically sorted order ?
> +
> +/* PCICR and PCICLKCR write enable magic key */
> +#define PCIC_WE_KEY		(0xa5 << 24)
> +
> +/* PCIC registers */
> +/* 0x0000 - 0x00ff mapped to PCI device configuration space */
> +#define PCIC_PCICR		0x100	/* PCI Control Register */
> +#define PCIC_PCICR_TRSB		BIT(9)	/* Target Read Single */
> +#define PCIC_PCICR_BSWP		BIT(8)	/* Target Byte Swap */
> +#define PCIC_PCICR_PLUP		BIT(7)	/* Enable PCI Pullup */
> +#define PCIC_PCICR_ARBM		BIT(6)	/* PCI Arbitration Mode */
> +#define PCIC_PCICR_MD10		BIT(5)	/* MD10 status */
> +#define PCIC_PCICR_MD9		BIT(4)	/* MD9 status */
> +#define PCIC_PCICR_SERR		BIT(3)	/* SERR output assert */
> +#define PCIC_PCICR_INTA		BIT(2)	/* INTA output assert */
> +#define PCIC_PCICR_PRST		BIT(1)	/* PCI Reset Assert */
> +#define PCIC_PCICR_CFIN		BIT(0)	/* Central Fun. Init Done */
> +
> +#define PCIC_PCILSR0		0x104	/* PCI Local Space Register0 */
> +#define PCIC_PCILSR1		0x108	/* PCI Local Space Register1 */
> +#define PCIC_PCILAR0		0x10c	/* PCI Local Addr Register1 */
> +#define PCIC_PCILAR1		0x110	/* PCI Local Addr Register1 */
> +#define PCIC_PCIINT		0x114	/* PCI Interrupt Register */
> +#define PCIC_PCIINTM		0x118	/* PCI Interrupt Mask */
> +#define PCIC_PCIALR		0x11c	/* Error Address Register */
> +#define PCIC_PCICLR		0x120	/* Error Command/Data */
> +#define PCIC_PCIAINT		0x130	/* Arbiter Interrupt Register */
> +#define PCIC_PCIAINTM		0x134	/* Arbiter Int. Mask Register */
> +#define PCIC_PCIBMLR		0x138	/* Error Bus Master Register */
> +#define PCIC_PCIDMABT		0x140	/* DMA Transfer Arb. Register */
> +#define PCIC_PCIPAR		0x1c0	/* PIO Address Register */
> +#define PCIC_PCIMBR		0x1c4	/* Memory Base Address */
> +#define PCIC_PCIIOBR		0x1c8	/* I/O Base Address Register */
> +
> +#define PCIC_PCIPINT		0x1cc	/* Power Mgmnt Int. Register */
> +#define PCIC_PCIPINT_D3		BIT(1)	/* D3 Pwr Mgmt. Interrupt */
> +#define PCIC_PCIPINT_D0		BIT(0)	/* D0 Pwr Mgmt. Interrupt */
> +
> +#define PCIC_PCIPINTM		0x1d0	/* Power Mgmnt Mask Register */
> +#define PCIC_PCICLKR		0x1d4	/* Clock Ctrl. Register */
> +#define PCIC_PCIBCR1		0x1e0	/* Memory BCR1 Register */
> +#define PCIC_PCIBCR2		0x1e4	/* Memory BCR2 Register */
> +#define PCIC_PCIWCR1		0x1e8	/* Wait Control 1 Register */
> +#define PCIC_PCIWCR2		0x1ec	/* Wait Control 2 Register */
> +#define PCIC_PCIWCR3		0x1f0	/* Wait Control 3 Register */
> +#define PCIC_PCIMCR		0x1f4	/* Memory Control Register */
> +#define PCIC_PCIBCR3		0x1f8	/* Memory BCR3 Register */
> +#define PCIC_PCIPDR		0x220	/* Port IO Data Register */
> +
> +/* PCI IDs */
> +/* Hitachi is the company that led to Renesas. */
> +/* The SH7751 was designed by Hitachi, so it has a Hitachi ID. */
multi-line comments way ?
> +#define PCI_VENDOR_ID_HITACHI	0x1054
> +#define PCI_DEVICE_ID_SH7751	0x3505
> +#define PCI_DEVICE_ID_SH7751R	0x350e
> +
> +/* BSC registers */
> +/* Copy BSC setting to PCI BSC */
> +#define BSC_BCR1		0x0000
> +#define BSC_BCR1_SLAVE		BIT(30)
> +#define BSC_BCR1_BRQEN		BIT(19)
> +#define BSC_BCR2		0x0004
> +#define BSC_BCR3		0x0050
> +#define BSC_WCR1		0x0008
> +#define BSC_WCR2		0x000c
> +#define BSC_WCR3		0x0010
> +#define BSC_MCR			0x0014
> +#define BSC_MCR_MRSET		BIT(30)
> +#define BSC_MCR_RFSH		BIT(2)
> +
> +/* PCIC access wrapper */
> +#define pcic_writel(val, base, reg)	writel(val, base + (reg))
> +#define pcic_readl(base, reg)		readl(base + (reg))
Do you really needed these new macros ? Is it for better readability ?
> +/*
> + * We need to avoid collisions with `mirrored' VGA ports
> + * and other strange ISA hardware, so we always want the
> + * addresses to be allocated in the 0x000-0x0ff region
> + * modulo 0x400.
> + */
> +#define IO_REGION_BASE 0x1000
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	resource_size_t start = res->start;
> +
> +	if (res->flags & IORESOURCE_IO) {
> +		if (start < PCIBIOS_MIN_IO + IO_REGION_BASE)
> +			start = PCIBIOS_MIN_IO + IO_REGION_BASE;
> +
> +		/*
> +		 * Put everything into 0x00-0xff region modulo 0x400.
> +		 */
single line comment would work. no ?
> +		if (start & 0x300)
> +			start = (start + 0x3ff) & ~0x3ff;
> +	}
> +
> +	return start;
> +}
> +
> +static int setup_pci_bsc(struct device *dev, void __iomem *pcic,
> +			 void __iomem *bsc, unsigned int area, bool bcr3)
> +{
> +	u32 word;
> +
> +	word = __raw_readl(bsc + BSC_BCR1);
> +	/* check BCR for SDRAM in area */
> +	if (((word >> area) & 1) == 0) {
> +		dev_err(dev, "Area %u is not configured for SDRAM. BCR1=0x%x\n",
> +			area, word);
> +		return -EINVAL;
> +	}
> +	word |= BSC_BCR1_SLAVE;		/* PCIC BSC is slave only */
> +	pcic_writel(word, pcic, PCIC_PCIBCR1);
> +
> +	word = __raw_readw(bsc + BSC_BCR2);
> +	/* check BCR2 for 32bit SDRAM interface*/
> +	if (((word >> (area << 1)) & 0x3) != 0x3) {
> +		dev_err(dev, "Area %u is not 32 bit SDRAM. BCR2=0x%x\n",
> +			area, word);
> +		return -EINVAL;
> +	}
> +	pcic_writel(word, pcic, PCIC_PCIBCR2);
> +
> +	if (bcr3) {
> +		/* BCR3 have only SH7751R */
> +		word = __raw_readw(bsc + BSC_BCR3);
> +		pcic_writel(word, pcic, PCIC_PCIBCR3);
> +	}
> +
> +	/* configure the wait control registers */
> +	word = __raw_readl(bsc + BSC_WCR1);
> +	pcic_writel(word, pcic, PCIC_PCIWCR1);
> +	word = __raw_readl(bsc + BSC_WCR2);
> +	pcic_writel(word, pcic, PCIC_PCIWCR2);
> +	word = __raw_readl(bsc + BSC_WCR3);
> +	pcic_writel(word, pcic, PCIC_PCIWCR3);
> +	word = __raw_readl(bsc + BSC_MCR);
> +	/* Clear MRSET and RFSH bit */
> +	word &= ~(BSC_MCR_MRSET | BSC_MCR_RFSH);
> +	pcic_writel(word, pcic, PCIC_PCIMCR);
> +
> +	return 0;
> +}
> +
> +#define NUM_AREA 7
> +static int set_pci_ranges(struct device *dev,
> +			  void __iomem *pcic, void __iomem *bsc, bool bcr3)
> +{
> +	struct resource_entry *dma, *tmp;
> +	struct pci_host_bridge *bridge;
> +	u32 bsc_done[NUM_AREA];
> +	unsigned int la;
> +
> +	bridge = dev_get_drvdata(dev);
> +	pcic_writel(0, pcic, PCIC_PCILAR0);
> +	pcic_writel(0, pcic, PCIC_PCILAR1);
> +	la = 0;
> +	memset(&bsc_done, 0, sizeof(bsc_done));
> +	resource_list_for_each_entry_safe(dma, tmp, &bridge->dma_ranges) {
> +		struct resource *res = dma->res;
> +		unsigned int area;
> +		u32 word;
> +
> +		switch (resource_type(res)) {
> +		case IORESOURCE_IO:
> +			/* BAR0 is I/O space */
> +			word = res->start | 1;
> +			pcic_writel(word, pcic, PCI_BASE_ADDRESS_0);
> +			word = pcic_readl(pcic, PCI_COMMAND);
> +			word |= PCI_COMMAND_IO;
> +			pcic_writel(word, pcic, PCI_COMMAND);
> +			break;
> +		case IORESOURCE_MEM:
> +			if (la > 4) {
> +				dev_err(dev, "Invalid range definition.\n");
> +				return -EINVAL;
> +			}
> +			area = (res->start >> 26) & 0x07;
> +			word = res->end - res->start;
> +			if (area >= NUM_AREA) {
> +				/* Area 7 is reserved. */
> +				dev_info(dev, "Invalid local address 0x%08x. Ignore it.\n",
> +					 res->start);
> +				break;
> +			}
> +			pcic_writel(res->start, pcic, PCI_BASE_ADDRESS_1 + la);
> +			/* if dummy entry, skip BSC setup */
> +			if (word < 4)
> +				break;
> +			/* BAR1 is local area 0, BAR2 is local area 1 */
> +			pcic_writel(word, pcic, PCIC_PCILSR0 + la);
> +			word = P2SEGADDR(res->start);
> +			pcic_writel(word, pcic, PCIC_PCILAR0 + la);
> +			la += 4;
> +			if (!bsc_done[area]) {
> +				/* check BCR for SDRAM in specified area. And setup PCI BSC. */
> +				if (setup_pci_bsc(dev, pcic, bsc, area, bcr3))
> +					return -EINVAL;
> +				bsc_done[area] = 1;
> +			}
> +			break;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int sh7751_pci_probe(struct platform_device *pdev)
> +{
> +	struct resource *res, *bscres;
> +	void __iomem *pcic;
> +	void __iomem *bsc;
> +	u16 vid, did;
> +	u32 word;
> +	int ret;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (IS_ERR(res))
> +		return PTR_ERR(res);
> +	pcic = ioremap(res->start, res->end - res->start + 1);
Can you consider using devm_platform_ioremap_resource() API ?
> +	bscres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	bsc = devm_ioremap_resource(&pdev->dev, bscres);
> +	if (IS_ERR(bsc))
> +		return PTR_ERR(bsc);
Same as above.
> +	/* check for SH7751/SH7751R hardware */
> +	word = pcic_readl(pcic, PCI_VENDOR_ID);
> +	vid = word & 0xffff;
> +	did = word >> 16;
> +	if ((vid != PCI_VENDOR_ID_HITACHI) ||
> +	    ((did != PCI_DEVICE_ID_SH7751) &&
> +	     (did != PCI_DEVICE_ID_SH7751R))) {
> +		dev_err(&pdev->dev, "This is not an SH7751(R)\n");
error handling missing iounmap(pcic)
> +		return -ENODEV;
> +	}
> +	dev_info(&pdev->dev, "PCI core found at %pR\n", res);
> +
> +	/* Set the BCR's to enable PCI access */
> +	word = __raw_readl(bsc + BSC_BCR1);
> +	word |= BSC_BCR1_BRQEN;
> +	__raw_writel(word, bsc + BSC_BCR1);
> +
> +	/* Turn the clocks back on (not done in reset)*/
> +	pcic_writel(PCIC_WE_KEY | 0, pcic, PCIC_PCICLKR);
> +	/* Clear Powerdown IRQ's (not done in reset) */
> +	word = PCIC_PCIPINT_D3 | PCIC_PCIPINT_D0;
> +	pcic_writel(word, pcic, PCIC_PCIPINT);
> +
> +	/* set the command/status */
> +	word = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
> +		PCI_COMMAND_PARITY | PCI_COMMAND_WAIT;
> +	pcic_writel(word, pcic, PCI_COMMAND);
> +
> +	/* define this host as the host bridge */
> +	word = PCI_BASE_CLASS_BRIDGE << 24;
> +	pcic_writel(word, pcic, PCI_CLASS_REVISION);
> +
> +	ret = pci_host_common_probe(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Initialize failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* Set IO and Mem windows to local address */
> +	if (set_pci_ranges(&pdev->dev, pcic, bsc,
> +			   did == PCI_DEVICE_ID_SH7751R))
> +		return -EINVAL;
error handling to call pci_host_common_remove() ?
> +	pcic_writel(0, pcic, PCIC_PCIIOBR);
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "renesas,bus-arbit-round-robin"))
> +		word |= BIT(0);
> +	else
> +		word = 0;
> +	pcic_writel(word, pcic, PCIC_PCIDMABT);
> +
> +	/* SH7751 init done, set central function init complete */
> +	/* use round robin mode to stop a device starving/overrunning */
multi-line comment ?
> +	word = PCIC_PCICR_CFIN | PCIC_PCICR_ARBM;
> +	pcic_writel(PCIC_WE_KEY | word, pcic, PCIC_PCICR);
> +
> +	return 0;
> +}
> +
> +/*
> + * Direct access to PCI hardware...
> + */
> +#define CONFIG_CMD(bus, devfn, where) \
> +	(0x80000000 | (bus->number << 16) | (devfn << 8) | (where & ~3))
> +
> +static void __iomem *sh4_pci_map_bus(struct pci_bus *bus,
> +				     unsigned int devfn, int where)
> +{
> +	struct pci_config_window *cfg = bus->sysdata;
> +	void __iomem *pcic = (void __iomem *)cfg->res.start;
> +
> +	pcic_writel(CONFIG_CMD(bus, devfn, where), pcic, PCIC_PCIPAR);
> +	return pcic + PCIC_PCIPDR;
> +}
> +
> +static const struct pci_ecam_ops pci_sh7751_bus_ops = {
> +	.pci_ops	= {
> +		.map_bus = sh4_pci_map_bus,
> +		.read    = pci_generic_config_read32,
> +		.write   = pci_generic_config_write32,
> +	}
> +};
> +
> +static const struct of_device_id sh7751_pci_of_match[] = {
> +	{ .compatible = "renesas,sh7751-pci",
> +	  .data = &pci_sh7751_bus_ops },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sh7751_pci_of_match);
> +
> +static struct platform_driver sh7751_pci_driver = {
> +	.driver = {
> +		.name = "sh7751-pci",
> +		.of_match_table = sh7751_pci_of_match,
> +	},
> +	.probe = sh7751_pci_probe,
> +};
> +module_platform_driver(sh7751_pci_driver);
> +
> +MODULE_DESCRIPTION("SH7751 PCI driver");
John Paul Adrian Glaubitz May 18, 2024, 9:08 a.m. UTC | #4
Hi Yoshinori,

On Thu, 2024-04-04 at 14:14 +0900, Yoshinori Sato wrote:
> Sorry. previus mail is thread broken.
> 
> This is an updated version of something I wrote about 7 years ago.
> Minimum support for R2D-plus and LANDISK.
> I think R2D-1 will work if you add AX88796 to dts.
> And board-specific functions and SCI's SPI functions are not supported.
> 
> You can get it working with qemu found here.
> https://gitlab.com/yoshinori.sato/qemu/-/tree/landisk
> 
> v7 changes.
> - sh/kernel/setup.c: fix kernel parameter handling.
> - clk-sh7750.c: cleanup.
> - sh_tmu.c: cleanup.
> - irq-renesas-sh7751.c: IPR definition move to code.
> - irq-renesas-sh7751irl.c: update register definition.
> - pci-sh7751.c: Register initialization fix. 
> - sm501 and sm501fb: Re-design Device Tree properties.

Could you push your v7 version to your Gitlab [1] repository so I can fetch
it from there?

Thanks,
Adrian

> [1] https://gitlab.com/yoshinori.sato/linux
Yoshinori Sato May 20, 2024, 1:06 p.m. UTC | #5
On Sat, 18 May 2024 18:08:30 +0900,
John Paul Adrian Glaubitz wrote:
> 
> Hi Yoshinori,
> 
> On Thu, 2024-04-04 at 14:14 +0900, Yoshinori Sato wrote:
> > Sorry. previus mail is thread broken.
> > 
> > This is an updated version of something I wrote about 7 years ago.
> > Minimum support for R2D-plus and LANDISK.
> > I think R2D-1 will work if you add AX88796 to dts.
> > And board-specific functions and SCI's SPI functions are not supported.
> > 
> > You can get it working with qemu found here.
> > https://gitlab.com/yoshinori.sato/qemu/-/tree/landisk
> > 
> > v7 changes.
> > - sh/kernel/setup.c: fix kernel parameter handling.
> > - clk-sh7750.c: cleanup.
> > - sh_tmu.c: cleanup.
> > - irq-renesas-sh7751.c: IPR definition move to code.
> > - irq-renesas-sh7751irl.c: update register definition.
> > - pci-sh7751.c: Register initialization fix. 
> > - sm501 and sm501fb: Re-design Device Tree properties.
> 
> Could you push your v7 version to your Gitlab [1] repository so I can fetch
> it from there?

updated it.
I'll be posting v8 soon.

> Thanks,
> Adrian
> 
> > [1] https://gitlab.com/yoshinori.sato/linux
> 
> -- 
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer
> `. `'   Physicist
>   `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
John Paul Adrian Glaubitz May 20, 2024, 3:25 p.m. UTC | #6
Hi Yoshinori,

On Mon, 2024-05-20 at 22:06 +0900, Yoshinori Sato wrote:
> On Sat, 18 May 2024 18:08:30 +0900,
> John Paul Adrian Glaubitz wrote:
> > 
> > Hi Yoshinori,
> > 
> > On Thu, 2024-04-04 at 14:14 +0900, Yoshinori Sato wrote:
> > > Sorry. previus mail is thread broken.
> > > 
> > > This is an updated version of something I wrote about 7 years ago.
> > > Minimum support for R2D-plus and LANDISK.
> > > I think R2D-1 will work if you add AX88796 to dts.
> > > And board-specific functions and SCI's SPI functions are not supported.
> > > 
> > > You can get it working with qemu found here.
> > > https://gitlab.com/yoshinori.sato/qemu/-/tree/landisk
> > > 
> > > v7 changes.
> > > - sh/kernel/setup.c: fix kernel parameter handling.
> > > - clk-sh7750.c: cleanup.
> > > - sh_tmu.c: cleanup.
> > > - irq-renesas-sh7751.c: IPR definition move to code.
> > > - irq-renesas-sh7751irl.c: update register definition.
> > > - pci-sh7751.c: Register initialization fix. 
> > > - sm501 and sm501fb: Re-design Device Tree properties.
> > 
> > Could you push your v7 version to your Gitlab [1] repository so I can fetch
> > it from there?
> 
> updated it.

Thanks!

> I'll be posting v8 soon.

Sounds good! Maybe we can start merging the patches that contain fixes only
and that have already been reviewed. This way, we can reduce the overall size
of the series a bit.

Adrian
Geert Uytterhoeven May 21, 2024, 7:19 a.m. UTC | #7
On Mon, May 20, 2024 at 5:25 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Mon, 2024-05-20 at 22:06 +0900, Yoshinori Sato wrote:
> > I'll be posting v8 soon.
>
> Sounds good! Maybe we can start merging the patches that contain fixes only
> and that have already been reviewed. This way, we can reduce the overall size
> of the series a bit.

+1

Gr{oetje,eeting}s,

                        Geert