[RFC PATCH v2 06/30] drivers/pci: SH7751 PCI Host bridge header.

[RFC PATCH v2 06/30] drivers/pci: SH7751 PCI Host bridge header.

[Yoshinori Sato]

This file move from SH specific directory "arch/sh/drivers/pci/pci-sh7751.h"

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 drivers/pci/controller/pci-sh7751.h | 270 ++++++++++++++++++++++++++++
 1 file changed, 270 insertions(+)
 create mode 100644 drivers/pci/controller/pci-sh7751.h

diff --git a/drivers/pci/controller/pci-sh7751.h b/drivers/pci/controller/pci-sh7751.h
new file mode 100644
index 000000000000..9b7de8243e92
--- /dev/null
+++ b/drivers/pci/controller/pci-sh7751.h
@@ -0,0 +1,270 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *	Low-Level PCI Support for SH7751 targets
+ *
+ *  Dustin McIntire (dustin@sensoria.com) (c) 2001
+ *  Paul Mundt (lethal@linux-sh.org) (c) 2003
+ *
+ *  May be copied or modified under the terms of the GNU General Public
+ *  License.  See linux/COPYING for more information.
+ *
+ */
+
+#ifndef _PCI_SH7751_H_
+#define _PCI_SH7751_H_
+
+#define SH4_PCICR		0x100		/* PCI Control Register */
+  #define SH4_PCICR_PREFIX	  0xA5000000	/* CR prefix for write */
+  #define SH4_PCICR_FTO		  0x00000400	/* TRDY/IRDY Enable */
+  #define SH4_PCICR_TRSB	  0x00000200	/* Target Read Single */
+  #define SH4_PCICR_BSWP	  0x00000100	/* Target Byte Swap */
+  #define SH4_PCICR_PLUP	  0x00000080	/* Enable PCI Pullup */
+  #define SH4_PCICR_ARBM	  0x00000040	/* PCI Arbitration Mode */
+  #define SH4_PCICR_MD		  0x00000030	/* MD9 and MD10 status */
+  #define SH4_PCICR_SERR	  0x00000008	/* SERR output assert */
+  #define SH4_PCICR_INTA	  0x00000004	/* INTA output assert */
+  #define SH4_PCICR_PRST	  0x00000002	/* PCI Reset Assert */
+  #define SH4_PCICR_CFIN	  0x00000001	/* Central Fun. Init Done */
+#define SH4_PCILSR0		0x104		/* PCI Local Space Register0 */
+#define SH4_PCILSR1		0x108		/* PCI Local Space Register1 */
+#define SH4_PCILAR0		0x10C		/* PCI Local Addr Register1 */
+#define SH4_PCILAR1		0x110		/* PCI Local Addr Register1 */
+#define SH4_PCIINT		0x114		/* PCI Interrupt Register */
+  #define SH4_PCIINT_MLCK	  0x00008000	/* Master Lock Error */
+  #define SH4_PCIINT_TABT	  0x00004000	/* Target Abort Error */
+  #define SH4_PCIINT_TRET	  0x00000200	/* Target Retry Error */
+  #define SH4_PCIINT_MFDE	  0x00000100	/* Master Func. Disable Error */
+  #define SH4_PCIINT_PRTY	  0x00000080	/* Address Parity Error */
+  #define SH4_PCIINT_SERR	  0x00000040	/* SERR Detection Error */
+  #define SH4_PCIINT_TWDP	  0x00000020	/* Tgt. Write Parity Error */
+  #define SH4_PCIINT_TRDP	  0x00000010	/* Tgt. Read Parity Err Det. */
+  #define SH4_PCIINT_MTABT	  0x00000008	/* Master-Tgt. Abort Error */
+  #define SH4_PCIINT_MMABT	  0x00000004	/* Master-Master Abort Error */
+  #define SH4_PCIINT_MWPD	  0x00000002	/* Master Write PERR Detect */
+  #define SH4_PCIINT_MRPD	  0x00000001	/* Master Read PERR Detect */
+#define SH4_PCIINTM		0x118		/* PCI Interrupt Mask */
+  #define SH4_PCIINTM_TTADIM	  BIT(14)	/* Target-target abort interrupt */
+  #define SH4_PCIINTM_TMTOIM	  BIT(9)	/* Target retry timeout */
+  #define SH4_PCIINTM_MDEIM	  BIT(8)	/* Master function disable error */
+  #define SH4_PCIINTM_APEDIM	  BIT(7)	/* Address parity error detection */
+  #define SH4_PCIINTM_SDIM	  BIT(6)	/* SERR detection */
+  #define SH4_PCIINTM_DPEITWM	  BIT(5)	/* Data parity error for target write */
+  #define SH4_PCIINTM_PEDITRM	  BIT(4)	/* PERR detection for target read */
+  #define SH4_PCIINTM_TADIMM	  BIT(3)	/* Target abort for master */
+  #define SH4_PCIINTM_MADIMM	  BIT(2)	/* Master abort for master */
+  #define SH4_PCIINTM_MWPDIM	  BIT(1)	/* Master write data parity error */
+  #define SH4_PCIINTM_MRDPEIM	  BIT(0)	/* Master read data parity error */
+#define SH4_PCIALR		0x11C		/* Error Address Register */
+#define SH4_PCICLR		0x120		/* Error Command/Data */
+  #define SH4_PCICLR_MPIO	  0x80000000
+  #define SH4_PCICLR_MDMA0	  0x40000000	/* DMA0 Transfer Error */
+  #define SH4_PCICLR_MDMA1	  0x20000000	/* DMA1 Transfer Error */
+  #define SH4_PCICLR_MDMA2	  0x10000000	/* DMA2 Transfer Error */
+  #define SH4_PCICLR_MDMA3	  0x08000000	/* DMA3 Transfer Error */
+  #define SH4_PCICLR_TGT	  0x04000000	/* Target Transfer Error */
+  #define SH4_PCICLR_CMDL	  0x0000000F	/* PCI Command at Error */
+#define SH4_PCIAINT		0x130		/* Arbiter Interrupt Register */
+  #define SH4_PCIAINT_MBKN	  0x00002000	/* Master Broken Interrupt */
+  #define SH4_PCIAINT_TBTO	  0x00001000	/* Target Bus Time Out */
+  #define SH4_PCIAINT_MBTO	  0x00000800	/* Master Bus Time Out */
+  #define SH4_PCIAINT_TABT	  0x00000008	/* Target Abort */
+  #define SH4_PCIAINT_MABT	  0x00000004	/* Master Abort */
+  #define SH4_PCIAINT_RDPE	  0x00000002	/* Read Data Parity Error */
+  #define SH4_PCIAINT_WDPE	  0x00000001	/* Write Data Parity Error */
+#define SH4_PCIAINTM            0x134		/* Arbiter Int. Mask Register */
+#define SH4_PCIBMLR		0x138		/* Error Bus Master Register */
+  #define SH4_PCIBMLR_REQ4	  0x00000010	/* REQ4 bus master at error */
+  #define SH4_PCIBMLR_REQ3	  0x00000008	/* REQ3 bus master at error */
+  #define SH4_PCIBMLR_REQ2	  0x00000004	/* REQ2 bus master at error */
+  #define SH4_PCIBMLR_REQ1	  0x00000002	/* REQ1 bus master at error */
+  #define SH4_PCIBMLR_REQ0	  0x00000001	/* REQ0 bus master at error */
+#define SH4_PCIDMABT		0x140		/* DMA Transfer Arb. Register */
+  #define SH4_PCIDMABT_RRBN	  0x00000001	/* DMA Arbitor Round-Robin */
+#define SH4_PCIDPA0		0x180		/* DMA0 Transfer Addr. */
+#define SH4_PCIDLA0		0x184		/* DMA0 Local Addr. */
+#define SH4_PCIDTC0		0x188		/* DMA0 Transfer Cnt. */
+#define SH4_PCIDCR0		0x18C		/* DMA0 Control Register */
+  #define SH4_PCIDCR_ALGN	  0x00000600	/* DMA Alignment Mode */
+  #define SH4_PCIDCR_MAST	  0x00000100	/* DMA Termination Type */
+  #define SH4_PCIDCR_INTM	  0x00000080	/* DMA Interrupt Done Mask*/
+  #define SH4_PCIDCR_INTS	  0x00000040	/* DMA Interrupt Done Status */
+  #define SH4_PCIDCR_LHLD	  0x00000020	/* Local Address Control */
+  #define SH4_PCIDCR_PHLD	  0x00000010	/* PCI Address Control*/
+  #define SH4_PCIDCR_IOSEL	  0x00000008	/* PCI Address Space Type */
+  #define SH4_PCIDCR_DIR	  0x00000004	/* DMA Transfer Direction */
+  #define SH4_PCIDCR_STOP	  0x00000002	/* Force DMA Stop */
+  #define SH4_PCIDCR_STRT	  0x00000001	/* DMA Start */
+#define SH4_PCIDPA1		0x190		/* DMA1 Transfer Addr. */
+#define SH4_PCIDLA1		0x194		/* DMA1 Local Addr. */
+#define SH4_PCIDTC1		0x198		/* DMA1 Transfer Cnt. */
+#define SH4_PCIDCR1		0x19C		/* DMA1 Control Register */
+#define SH4_PCIDPA2		0x1A0		/* DMA2 Transfer Addr. */
+#define SH4_PCIDLA2		0x1A4		/* DMA2 Local Addr. */
+#define SH4_PCIDTC2		0x1A8		/* DMA2 Transfer Cnt. */
+#define SH4_PCIDCR2		0x1AC		/* DMA2 Control Register */
+#define SH4_PCIDPA3		0x1B0		/* DMA3 Transfer Addr. */
+#define SH4_PCIDLA3		0x1B4		/* DMA3 Local Addr. */
+#define SH4_PCIDTC3		0x1B8		/* DMA3 Transfer Cnt. */
+#define SH4_PCIDCR3		0x1BC		/* DMA3 Control Register */
+#define SH4_PCIPAR		0x1C0		/* PIO Address Register */
+  #define SH4_PCIPAR_CFGEN	  0x80000000	/* Configuration Enable */
+  #define SH4_PCIPAR_BUSNO	  0x00FF0000	/* Config. Bus Number */
+  #define SH4_PCIPAR_DEVNO	  0x0000FF00	/* Config. Device Number */
+  #define SH4_PCIPAR_REGAD	  0x000000FC	/* Register Address Number */
+#define SH4_PCIMBR		0x1C4		/* Memory Base Address */
+  #define SH4_PCIMBR_MASK	  0xFF000000	/* Memory Space Mask */
+  #define SH4_PCIMBR_LOCK	  0x00000001	/* Lock Memory Space */
+#define SH4_PCIIOBR		0x1C8		/* I/O Base Address Register */
+  #define SH4_PCIIOBR_MASK	  0xFFFC0000	/* IO Space Mask */
+  #define SH4_PCIIOBR_LOCK	  0x00000001	/* Lock IO Space */
+#define SH4_PCIPINT		0x1CC		/* Power Mgmnt Int. Register */
+  #define SH4_PCIPINT_D3	  0x00000002	/* D3 Pwr Mgmt. Interrupt */
+  #define SH4_PCIPINT_D0	  0x00000001	/* D0 Pwr Mgmt. Interrupt */
+#define SH4_PCIPINTM		0x1D0		/* Power Mgmnt Mask Register */
+#define SH4_PCICLKR		0x1D4		/* Clock Ctrl. Register */
+  #define SH4_PCICLKR_PCSTP	  0x00000002	/* PCI Clock Stop */
+  #define SH4_PCICLKR_BCSTP	  0x00000001	/* BCLK Clock Stop */
+/* For definitions of BCR, MCR see ... */
+#define SH4_PCIBCR1		0x1E0		/* Memory BCR1 Register */
+  #define SH4_PCIMBR0		SH4_PCIBCR1
+#define SH4_PCIBCR2		0x1E4		/* Memory BCR2 Register */
+  #define SH4_PCIMBMR0		SH4_PCIBCR2
+#define SH4_PCIWCR1		0x1E8		/* Wait Control 1 Register */
+#define SH4_PCIWCR2		0x1EC		/* Wait Control 2 Register */
+#define SH4_PCIWCR3		0x1F0		/* Wait Control 3 Register */
+  #define SH4_PCIMBR2		SH4_PCIWCR3
+#define SH4_PCIMCR		0x1F4		/* Memory Control Register */
+#define SH4_PCIBCR3		0x1f8		/* Memory BCR3 Register */
+#define SH4_PCIPCTR             0x200		/* Port Control Register */
+  #define SH4_PCIPCTR_P2EN	  0x000400000	/* Port 2 Enable */
+  #define SH4_PCIPCTR_P1EN	  0x000200000	/* Port 1 Enable */
+  #define SH4_PCIPCTR_P0EN	  0x000100000	/* Port 0 Enable */
+  #define SH4_PCIPCTR_P2UP	  0x000000020	/* Port2 Pull Up Enable */
+  #define SH4_PCIPCTR_P2IO	  0x000000010	/* Port2 Output Enable */
+  #define SH4_PCIPCTR_P1UP	  0x000000008	/* Port1 Pull Up Enable */
+  #define SH4_PCIPCTR_P1IO	  0x000000004	/* Port1 Output Enable */
+  #define SH4_PCIPCTR_P0UP	  0x000000002	/* Port0 Pull Up Enable */
+  #define SH4_PCIPCTR_P0IO	  0x000000001	/* Port0 Output Enable */
+#define SH4_PCIPDTR		0x204		/* Port Data Register */
+  #define SH4_PCIPDTR_PB5	  0x000000020	/* Port 5 Enable */
+  #define SH4_PCIPDTR_PB4	  0x000000010	/* Port 4 Enable */
+  #define SH4_PCIPDTR_PB3	  0x000000008	/* Port 3 Enable */
+  #define SH4_PCIPDTR_PB2	  0x000000004	/* Port 2 Enable */
+  #define SH4_PCIPDTR_PB1	  0x000000002	/* Port 1 Enable */
+  #define SH4_PCIPDTR_PB0	  0x000000001	/* Port 0 Enable */
+#define SH4_PCIPDR		0x220		/* Port IO Data Register */
+
+/* Platform Specific Values */
+#define SH7751_VENDOR_ID             0x1054
+#define SH7751_DEVICE_ID             0x3505
+#define SH7751R_DEVICE_ID            0x350e
+
+/* SH7751 Specific Values */
+#define SH7751_PCI_CONFIG_BASE	     0xFD000000  /* Config space base addr */
+#define SH7751_PCI_CONFIG_SIZE       0x1000000   /* Config space size */
+#define SH7751_PCI_MEMORY_BASE	     0xFD000000  /* Memory space base addr */
+#define SH7751_PCI_MEM_SIZE          0x01000000  /* Size of Memory window */
+#define SH7751_PCI_IO_BASE           0xFE240000  /* IO space base address */
+#define SH7751_PCI_IO_SIZE           0x40000     /* Size of IO window */
+
+#define SH7751_PCIREG_BASE           0xFE200000  /* PCI regs base address */
+
+#define SH7751_PCICONF0            0x0           /* PCI Config Reg 0 */
+  #define SH7751_PCICONF0_DEVID      0xFFFF0000  /* Device ID */
+  #define SH7751_PCICONF0_VNDID      0x0000FFFF  /* Vendor ID */
+#define SH7751_PCICONF1            0x4           /* PCI Config Reg 1 */
+  #define SH7751_PCICONF1_DPE        0x80000000  /* Data Parity Error */
+  #define SH7751_PCICONF1_SSE        0x40000000  /* System Error Status */
+  #define SH7751_PCICONF1_RMA        0x20000000  /* Master Abort */
+  #define SH7751_PCICONF1_RTA        0x10000000  /* Target Abort Rx Status */
+  #define SH7751_PCICONF1_STA        0x08000000  /* Target Abort Exec Status */
+  #define SH7751_PCICONF1_DEV        0x06000000  /* Timing Status */
+  #define SH7751_PCICONF1_DPD        0x01000000  /* Data Parity Status */
+  #define SH7751_PCICONF1_FBBC       0x00800000  /* Back 2 Back Status */
+  #define SH7751_PCICONF1_UDF        0x00400000  /* User Defined Status */
+  #define SH7751_PCICONF1_66M        0x00200000  /* 66Mhz Operation Status */
+  #define SH7751_PCICONF1_PM         0x00100000  /* Power Management Status */
+  #define SH7751_PCICONF1_PBBE       0x00000200  /* Back 2 Back Control */
+  #define SH7751_PCICONF1_SER        0x00000100  /* SERR Output Control */
+  #define SH7751_PCICONF1_WCC        0x00000080  /* Wait Cycle Control */
+  #define SH7751_PCICONF1_PER        0x00000040  /* Parity Error Response */
+  #define SH7751_PCICONF1_VPS        0x00000020  /* VGA Pallet Snoop */
+  #define SH7751_PCICONF1_MWIE       0x00000010  /* Memory Write+Invalidate */
+  #define SH7751_PCICONF1_SPC        0x00000008  /* Special Cycle Control */
+  #define SH7751_PCICONF1_BUM        0x00000004  /* Bus Master Control */
+  #define SH7751_PCICONF1_MES        0x00000002  /* Memory Space Control */
+  #define SH7751_PCICONF1_IOS        0x00000001  /* I/O Space Control */
+#define SH7751_PCICONF2            0x8           /* PCI Config Reg 2 */
+  #define SH7751_PCICONF2_BCC        0xFF000000  /* Base Class Code */
+  #define SH7751_PCICONF2_SCC        0x00FF0000  /* Sub-Class Code */
+  #define SH7751_PCICONF2_RLPI       0x0000FF00  /* Programming Interface */
+  #define SH7751_PCICONF2_REV        0x000000FF  /* Revision ID */
+#define SH7751_PCICONF3            0xC           /* PCI Config Reg 3 */
+  #define SH7751_PCICONF3_BIST7      0x80000000  /* Bist Supported */
+  #define SH7751_PCICONF3_BIST6      0x40000000  /* Bist Executing */
+  #define SH7751_PCICONF3_BIST3_0    0x0F000000  /* Bist Passed */
+  #define SH7751_PCICONF3_HD7        0x00800000  /* Single Function device */
+  #define SH7751_PCICONF3_HD6_0      0x007F0000  /* Configuration Layout */
+  #define SH7751_PCICONF3_LAT        0x0000FF00  /* Latency Timer */
+  #define SH7751_PCICONF3_CLS        0x000000FF  /* Cache Line Size */
+#define SH7751_PCICONF4            0x10          /* PCI Config Reg 4 */
+  #define SH7751_PCICONF4_BASE       0xFFFFFFFC  /* I/O Space Base Addr */
+  #define SH7751_PCICONF4_ASI        0x00000001  /* Address Space Type */
+#define SH7751_PCICONF5            0x14          /* PCI Config Reg 5 */
+  #define SH7751_PCICONF5_BASE       0xFFFFFFF0  /* Mem Space Base Addr */
+  #define SH7751_PCICONF5_LAP        0x00000008  /* Prefetch Enabled */
+  #define SH7751_PCICONF5_LAT        0x00000006  /* Local Memory type */
+  #define SH7751_PCICONF5_ASI        0x00000001  /* Address Space Type */
+#define SH7751_PCICONF6            0x18          /* PCI Config Reg 6 */
+  #define SH7751_PCICONF6_BASE       0xFFFFFFF0  /* Mem Space Base Addr */
+  #define SH7751_PCICONF6_LAP        0x00000008  /* Prefetch Enabled */
+  #define SH7751_PCICONF6_LAT        0x00000006  /* Local Memory type */
+  #define SH7751_PCICONF6_ASI        0x00000001  /* Address Space Type */
+/* PCICONF7 - PCICONF10 are undefined */
+#define SH7751_PCICONF11           0x2C          /* PCI Config Reg 11 */
+  #define SH7751_PCICONF11_SSID      0xFFFF0000  /* Subsystem ID */
+  #define SH7751_PCICONF11_SVID      0x0000FFFF  /* Subsystem Vendor ID */
+/* PCICONF12 is undefined */
+#define SH7751_PCICONF13           0x34          /* PCI Config Reg 13 */
+  #define SH7751_PCICONF13_CPTR      0x000000FF  /* PM function pointer */
+/* PCICONF14 is undefined */
+#define SH7751_PCICONF15           0x3C          /* PCI Config Reg 15 */
+  #define SH7751_PCICONF15_IPIN      0x000000FF  /* Interrupt Pin */
+#define SH7751_PCICONF16           0x40          /* PCI Config Reg 16 */
+  #define SH7751_PCICONF16_PMES      0xF8000000  /* PME Support */
+  #define SH7751_PCICONF16_D2S       0x04000000  /* D2 Support */
+  #define SH7751_PCICONF16_D1S       0x02000000  /* D1 Support */
+  #define SH7751_PCICONF16_DSI       0x00200000  /* Bit Device Init. */
+  #define SH7751_PCICONF16_PMCK      0x00080000  /* Clock for PME req. */
+  #define SH7751_PCICONF16_VER       0x00070000  /* PM Version */
+  #define SH7751_PCICONF16_NIP       0x0000FF00  /* Next Item Pointer */
+  #define SH7751_PCICONF16_CID       0x000000FF  /* Capability Identifier */
+#define SH7751_PCICONF17           0x44          /* PCI Config Reg 17 */
+  #define SH7751_PCICONF17_DATA      0xFF000000  /* Data field for PM */
+  #define SH7751_PCICONF17_PMES      0x00800000  /* PME Status */
+  #define SH7751_PCICONF17_DSCL      0x00600000  /* Data Scaling Value */
+  #define SH7751_PCICONF17_DSEL      0x001E0000  /* Data Select */
+  #define SH7751_PCICONF17_PMEN      0x00010000  /* PME Enable */
+  #define SH7751_PCICONF17_PWST      0x00000003  /* Power State */
+/* SH7751 Internal PCI Registers */
+
+/* Memory Control Registers */
+#define SH7751_BCR1                0x0000    /* Memory BCR1 Register */
+#define SH7751_BCR2                0x0004    /* Memory BCR2 Register */
+#define SH7751_BCR3                0x0050    /* Memory BCR3 Register */
+#define SH7751_WCR1                0x0008    /* Wait Control 1 Register */
+#define SH7751_WCR2                0x000C    /* Wait Control 2 Register */
+#define SH7751_WCR3                0x0010    /* Wait Control 3 Register */
+#define SH7751_MCR                 0x0014    /* Memory Control Register */
+
+/* General Memory Config Addresses */
+#define SH7751_CS0_BASE_ADDR       0x0
+#define SH7751_MEM_REGION_SIZE     0x04000000
+#define SH7751_CS1_BASE_ADDR       (SH7751_CS0_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+#define SH7751_CS2_BASE_ADDR       (SH7751_CS1_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+#define SH7751_CS3_BASE_ADDR       (SH7751_CS2_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+#define SH7751_CS4_BASE_ADDR       (SH7751_CS3_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+#define SH7751_CS5_BASE_ADDR       (SH7751_CS4_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+#define SH7751_CS6_BASE_ADDR       (SH7751_CS5_BASE_ADDR + SH7751_MEM_REGION_SIZE)
+
+#endif /* _PCI_SH7751_H_ */
-- 
2.39.2

Re: [RFC PATCH v2 06/30] drivers/pci: SH7751 PCI Host bridge header.

[Bjorn Helgaas]

On Wed, Sep 13, 2023 at 06:23:31PM +0900, Yoshinori Sato wrote:
> This file move from SH specific directory "arch/sh/drivers/pci/pci-sh7751.h"
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  drivers/pci/controller/pci-sh7751.h | 270 ++++++++++++++++++++++++++++
>  1 file changed, 270 insertions(+)
>  create mode 100644 drivers/pci/controller/pci-sh7751.h

If this is a move from arch/sh/drivers/pci/, shouldn't this diff show
both the removal and the addition?  Or even better, if you use "git
mv", won't it show the move directly, without a diff at all?

This file is only used in one place, so please just incorporate it
directly into pci-sh7751.c.  I think the git history would be a little
cleaner if you did this as a separate patch before moving it to
drivers/pci/.

If you're moving things into drivers/pci/, follow the subject line
conventions (see "git log --oneline drivers/pci/controller/"):

  - No period at end of subject line

  - Subject line begins with "PCI: <driver-tag>: <Verb> ...", e.g.,
    PCI: sh7751: Add SH7751 driver

> diff --git a/drivers/pci/controller/pci-sh7751.h b/drivers/pci/controller/pci-sh7751.h
> new file mode 100644
> index 000000000000..9b7de8243e92
> --- /dev/null
> +++ b/drivers/pci/controller/pci-sh7751.h
> @@ -0,0 +1,270 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *	Low-Level PCI Support for SH7751 targets
> + *
> + *  Dustin McIntire (dustin@sensoria.com) (c) 2001
> + *  Paul Mundt (lethal@linux-sh.org) (c) 2003
> + *
> + *  May be copied or modified under the terms of the GNU General Public
> + *  License.  See linux/COPYING for more information.

Unnecessary text, given the SPDX header above.

> + *

Spurious blank line.

> +/* Platform Specific Values */
> +#define SH7751_VENDOR_ID             0x1054
> +#define SH7751_DEVICE_ID             0x3505
> +#define SH7751R_DEVICE_ID            0x350e

Most of this file uses upper-case hex, so use it consistently.

Bjorn

[RFC PATCH v2 07/30] drivers/pci: SH7751 PCI Host bridge controller driver.

[Yoshinori Sato]

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 drivers/pci/controller/pci-sh7751.c | 338 ++++++++++++++++++++++++++++
 1 file changed, 338 insertions(+)
 create mode 100644 drivers/pci/controller/pci-sh7751.c

diff --git a/drivers/pci/controller/pci-sh7751.c b/drivers/pci/controller/pci-sh7751.c
new file mode 100644
index 000000000000..f6718c6b403b
--- /dev/null
+++ b/drivers/pci/controller/pci-sh7751.c
@@ -0,0 +1,338 @@
+// 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 <asm-generic/pci.h>
+#include "pci-sh7751.h"
+
+#define pcic_writel(val, reg) __raw_writel(val, pci_reg_base + (reg))
+#define pcic_readl(reg) __raw_readl(pci_reg_base + (reg))
+
+DEFINE_RAW_SPINLOCK(pci_config_lock);
+
+/*
+ * PCIC fixups
+ */
+
+#define PCIMCR_MRSET 0x40000000
+#define PCIMCR_RFSH  0x00000004
+
+/* board depend PCI bus fixups */
+static void __init julian_fixup(void __iomem *pci_reg_base, void __iomem *bcr)
+{
+	unsigned long bcr1, mcr;
+
+	bcr1 = __raw_readl(bcr + SH7751_BCR1);
+	bcr1 |= 0x00080000;	/* Enable Bit 19 BREQEN, set PCIC to slave */
+	pcic_writel(bcr1, SH4_PCIBCR1);
+
+	mcr = __raw_readl(bcr + SH7751_MCR);
+	mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
+	pcic_writel(mcr, SH4_PCIMCR);
+
+	pcic_writel(0x0c000000, SH7751_PCICONF5);
+	pcic_writel(0xd0000000, SH7751_PCICONF6);
+	pcic_writel(0x0c000000, SH4_PCILAR0);
+	pcic_writel(0x00000000, SH4_PCILAR1);
+}
+
+static void __init r2d_fixup(void __iomem *pci_reg_base, void __iomem *bcr)
+{
+	unsigned long bcr1, mcr;
+
+	bcr1 = ioread32(bcr + SH7751_BCR1);
+	bcr1 |= 0x40080000;	/* Enable Bit 19 BREQEN, set PCIC to slave */
+	pcic_writel(bcr1, SH4_PCIBCR1);
+
+	/* Enable all interrupts, so we known what to fix */
+	pcic_writel(0x0000c3ff, SH4_PCIINTM);
+	pcic_writel(0x0000380f, SH4_PCIAINTM);
+
+	pcic_writel(0xfb900047, SH7751_PCICONF1);
+	pcic_writel(0xab000001, SH7751_PCICONF4);
+
+	mcr = ioread32(bcr + SH7751_MCR);
+	mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
+	pcic_writel(mcr, SH4_PCIMCR);
+
+	pcic_writel(0x0c000000, SH7751_PCICONF5);
+	pcic_writel(0xd0000000, SH7751_PCICONF6);
+	pcic_writel(0x0c000000, SH4_PCILAR0);
+	pcic_writel(0x00000000, SH4_PCILAR1);
+}
+
+static const __initconst struct fixups {
+	char *compatible;
+	void (*fixup)(void __iomem *pci_reg_base, void __iomem *bcr);
+} fixup_list[] = {
+	{
+		.compatible = "iodata,julian-pci",
+		.fixup = julian_fixup,
+	},
+	{
+		.compatible = "renesas,r2d-pci",
+		.fixup = r2d_fixup,
+	},
+};
+
+static void pcic_fixups(struct device_node *np,
+		       void __iomem *pcic, void __iomem *bcr)
+{
+	int i;
+	const struct fixups *f = fixup_list;
+
+	for (i = 0; i < ARRAY_SIZE(fixup_list); i++) {
+		if (of_device_is_compatible(np, f->compatible)) {
+			f->fixup(pcic, bcr);
+			break;
+		}
+	}
+}
+
+/*
+ * Direct access to PCI hardware...
+ */
+#define CONFIG_CMD(bus, devfn, where) \
+	(0x80000000 | (bus->number << 16) | (devfn << 8) | (where & ~3))
+
+/*
+ * Functions for accessing PCI configuration space with type 1 accesses
+ */
+static int sh4_pci_read(struct pci_bus *bus, unsigned int devfn,
+			   int where, int size, u32 *val)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	void __iomem *pci_reg_base = (void __iomem *)cfg->res.start;
+	unsigned long flags;
+	u32 data;
+
+	/*
+	 * PCIPDR may only be accessed as 32 bit words,
+	 * so we must do byte alignment by hand
+	 */
+	raw_spin_lock_irqsave(&pci_config_lock, flags);
+	pcic_writel(CONFIG_CMD(bus, devfn, where), SH4_PCIPAR);
+	data = pcic_readl(SH4_PCIPDR);
+	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+
+	switch (size) {
+	case 1:
+		*val = (data >> ((where & 3) << 3)) & 0xff;
+		break;
+	case 2:
+		*val = (data >> ((where & 2) << 3)) & 0xffff;
+		break;
+	case 4:
+		*val = data;
+		break;
+	default:
+		return PCIBIOS_FUNC_NOT_SUPPORTED;
+	}
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+/*
+ * Since SH4 only does 32bit access we'll have to do a read,
+ * mask,write operation.
+ * We'll allow an odd byte offset, though it should be illegal.
+ */
+static int sh4_pci_write(struct pci_bus *bus, unsigned int devfn,
+			 int where, int size, u32 val)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	void __iomem *pci_reg_base = (void __iomem *)cfg->res.start;
+	unsigned long flags;
+	int shift;
+	u32 data;
+
+	raw_spin_lock_irqsave(&pci_config_lock, flags);
+	pcic_writel(CONFIG_CMD(bus, devfn, where), SH4_PCIPAR);
+	data = pcic_readl(SH4_PCIPDR);
+	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+
+	switch (size) {
+	case 1:
+		shift = (where & 3) << 3;
+		data &= ~(0xff << shift);
+		data |= ((val & 0xff) << shift);
+		break;
+	case 2:
+		shift = (where & 2) << 3;
+		data &= ~(0xffff << shift);
+		data |= ((val & 0xffff) << shift);
+		break;
+	case 4:
+		data = val;
+		break;
+	default:
+		return PCIBIOS_FUNC_NOT_SUPPORTED;
+	}
+
+	pcic_writel(data, SH4_PCIPDR);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int area_sdram_check(void __iomem *pci_reg_base,
+			    void __iomem *bcr,
+			    unsigned int area)
+{
+	unsigned long word;
+
+	word = __raw_readl(bcr + SH7751_BCR1);
+	/* check BCR for SDRAM in area */
+	if (((word >> area) & 1) == 0) {
+		pr_info("PCI: Area %d is not configured for SDRAM. BCR1=0x%lx\n",
+		       area, word);
+		return 0;
+	}
+	pcic_writel(word, SH4_PCIBCR1);
+
+	word = __raw_readw(bcr + SH7751_BCR2);
+	/* check BCR2 for 32bit SDRAM interface*/
+	if (((word >> (area << 1)) & 0x3) != 0x3) {
+		pr_info("PCI: Area %d is not 32 bit SDRAM. BCR2=0x%lx\n",
+			area, word);
+		return 0;
+	}
+	pcic_writel(word, SH4_PCIBCR2);
+
+	return 1;
+}
+
+static int sh7751_pci_probe(struct platform_device *pdev)
+{
+	struct resource *res, *wres;
+	u32 id;
+	u32 reg, word;
+	void __iomem *pci_reg_base;
+	void __iomem *bcr;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pci_reg_base = (void __iomem *)res->start;
+	if (IS_ERR(pci_reg_base))
+		return PTR_ERR(pci_reg_base);
+
+	wres = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (IS_ERR(wres))
+		return PTR_ERR(wres);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	bcr = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pci_reg_base))
+		return PTR_ERR(bcr);
+
+	/* check for SH7751/SH7751R hardware */
+	id = pcic_readl(SH7751_PCICONF0);
+	if (id != ((SH7751_DEVICE_ID << 16) | SH7751_VENDOR_ID) &&
+	    id != ((SH7751R_DEVICE_ID << 16) | SH7751_VENDOR_ID)) {
+		pr_warn("PCI: This is not an SH7751(R)\n");
+		return -ENODEV;
+	}
+	dev_info(&pdev->dev, "PCI core found at %p\n",
+		pci_reg_base);
+
+	/* Set the BCR's to enable PCI access */
+	reg = __raw_readl(bcr);
+	reg |= 0x80000;
+	__raw_writel(reg, bcr);
+
+	/* Turn the clocks back on (not done in reset)*/
+	pcic_writel(0, SH4_PCICLKR);
+	/* Clear Powerdown IRQ's (not done in reset) */
+	word = SH4_PCIPINT_D3 | SH4_PCIPINT_D0;
+	pcic_writel(word, SH4_PCIPINT);
+
+	/* set the command/status bits to:
+	 * Wait Cycle Control + Parity Enable + Bus Master +
+	 * Mem space enable
+	 */
+	word = SH7751_PCICONF1_WCC | SH7751_PCICONF1_PER |
+	       SH7751_PCICONF1_BUM | SH7751_PCICONF1_MES;
+	pcic_writel(word, SH7751_PCICONF1);
+
+	/* define this host as the host bridge */
+	word = PCI_BASE_CLASS_BRIDGE << 24;
+	pcic_writel(word, SH7751_PCICONF2);
+
+	/* Set IO and Mem windows to local address
+	 * Make PCI and local address the same for easy 1 to 1 mapping
+	 */
+	word = wres->end - wres->start - 1;
+	pcic_writel(word, SH4_PCILSR0);
+	/* Set the values on window 0 PCI config registers */
+	word = P2SEGADDR(wres->start);
+	pcic_writel(word, SH4_PCILAR0);
+	pcic_writel(word, SH7751_PCICONF5);
+
+	/* check BCR for SDRAM in specified area */
+	area_sdram_check(pci_reg_base, bcr, (wres->start >> 27) & 0x07);
+
+	/* configure the wait control registers */
+	word = __raw_readl(bcr + SH7751_WCR1);
+	pcic_writel(word, SH4_PCIWCR1);
+	word = __raw_readl(bcr + SH7751_WCR2);
+	pcic_writel(word, SH4_PCIWCR2);
+	word = __raw_readl(bcr + SH7751_WCR3);
+	pcic_writel(word, SH4_PCIWCR3);
+	word = __raw_readl(bcr + SH7751_MCR);
+	pcic_writel(word, SH4_PCIMCR);
+
+	pcic_fixups(pdev->dev.of_node, pci_reg_base, bcr);
+
+	/* SH7751 init done, set central function init complete */
+	/* use round robin mode to stop a device starving/overruning */
+	word = SH4_PCICR_PREFIX | SH4_PCICR_CFIN | SH4_PCICR_ARBM;
+	pcic_writel(word, SH4_PCICR);
+
+	return pci_host_common_probe(pdev);
+}
+
+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 *pci_reg_base = (void __iomem *)cfg->res.start;
+
+	pcic_writel(CONFIG_CMD(bus, devfn, where), SH4_PCIPAR);
+	return pci_reg_base + SH4_PCIPDR;
+}
+
+static const struct pci_ecam_ops pci_sh7751_bus_ops = {
+	.pci_ops	= {
+		.map_bus	= sh4_pci_map_bus,
+		.read		= sh4_pci_read,
+		.write		= sh4_pci_write,
+	}
+};
+
+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");
-- 
2.39.2

Re: [RFC PATCH v2 07/30] drivers/pci: SH7751 PCI Host bridge controller driver.

[Geert Uytterhoeven]

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:35 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pci/controller/pci-sh7751.c
> @@ -0,0 +1,338 @@
> +// 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 <asm-generic/pci.h>
> +#include "pci-sh7751.h"
> +
> +#define pcic_writel(val, reg) __raw_writel(val, pci_reg_base + (reg))
> +#define pcic_readl(reg) __raw_readl(pci_reg_base + (reg))
> +
> +DEFINE_RAW_SPINLOCK(pci_config_lock);
> +
> +/*
> + * PCIC fixups
> + */
> +
> +#define PCIMCR_MRSET 0x40000000
> +#define PCIMCR_RFSH  0x00000004
> +
> +/* board depend PCI bus fixups */
> +static void __init julian_fixup(void __iomem *pci_reg_base, void __iomem *bcr)

Please drop all the __init* annotations.
Although I no longer see invalid section warnings, all symbols tagged
with __init* are still referenced from sh7751_pci_probe(), eventually.

> +{
> +       unsigned long bcr1, mcr;
> +
> +       bcr1 = __raw_readl(bcr + SH7751_BCR1);
> +       bcr1 |= 0x00080000;     /* Enable Bit 19 BREQEN, set PCIC to slave */
> +       pcic_writel(bcr1, SH4_PCIBCR1);
> +
> +       mcr = __raw_readl(bcr + SH7751_MCR);
> +       mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
> +       pcic_writel(mcr, SH4_PCIMCR);
> +
> +       pcic_writel(0x0c000000, SH7751_PCICONF5);
> +       pcic_writel(0xd0000000, SH7751_PCICONF6);
> +       pcic_writel(0x0c000000, SH4_PCILAR0);
> +       pcic_writel(0x00000000, SH4_PCILAR1);
> +}
> +
> +static void __init r2d_fixup(void __iomem *pci_reg_base, void __iomem *bcr)
> +{
> +       unsigned long bcr1, mcr;
> +
> +       bcr1 = ioread32(bcr + SH7751_BCR1);
> +       bcr1 |= 0x40080000;     /* Enable Bit 19 BREQEN, set PCIC to slave */
> +       pcic_writel(bcr1, SH4_PCIBCR1);
> +
> +       /* Enable all interrupts, so we known what to fix */
> +       pcic_writel(0x0000c3ff, SH4_PCIINTM);
> +       pcic_writel(0x0000380f, SH4_PCIAINTM);
> +
> +       pcic_writel(0xfb900047, SH7751_PCICONF1);
> +       pcic_writel(0xab000001, SH7751_PCICONF4);
> +
> +       mcr = ioread32(bcr + SH7751_MCR);
> +       mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
> +       pcic_writel(mcr, SH4_PCIMCR);
> +
> +       pcic_writel(0x0c000000, SH7751_PCICONF5);
> +       pcic_writel(0xd0000000, SH7751_PCICONF6);
> +       pcic_writel(0x0c000000, SH4_PCILAR0);
> +       pcic_writel(0x00000000, SH4_PCILAR1);
> +}
> +
> +static const __initconst struct fixups {
> +       char *compatible;
> +       void (*fixup)(void __iomem *pci_reg_base, void __iomem *bcr);
> +} fixup_list[] = {
> +       {
> +               .compatible = "iodata,julian-pci",
> +               .fixup = julian_fixup,
> +       },
> +       {
> +               .compatible = "renesas,r2d-pci",
> +               .fixup = r2d_fixup,
> +       },
> +};

These fixups seem to be board-specific instead of specific to the
PCI block in the SoCs on these boards.

I see three options to handle this in a more appropriate way:
  1. Handle this in the bootloader.
     Not an attractive solution, as not everyone can/wants to update
     the bootloader,
  2. Use of_machine_is_compatible() in a platform-specific quirk
     handler, outside the PCI driver,
  3. Move the common parts into sh7751_pci_probe(), and the
     handle the differences through DT topology analysis and/or
     properties in DT.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH v2 07/30] drivers/pci: SH7751 PCI Host bridge controller driver.

[Yoshinori Sato]

On Tue, 19 Sep 2023 00:32:46 +0900,
Geert Uytterhoeven wrote:
> 
> Hi Sato-san,
> 
> On Wed, Sep 13, 2023 at 11:35 AM Yoshinori Sato
> <ysato@users.sourceforge.jp> wrote:
> > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/drivers/pci/controller/pci-sh7751.c
> > @@ -0,0 +1,338 @@
> > +// 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 <asm-generic/pci.h>
> > +#include "pci-sh7751.h"
> > +
> > +#define pcic_writel(val, reg) __raw_writel(val, pci_reg_base + (reg))
> > +#define pcic_readl(reg) __raw_readl(pci_reg_base + (reg))
> > +
> > +DEFINE_RAW_SPINLOCK(pci_config_lock);
> > +
> > +/*
> > + * PCIC fixups
> > + */
> > +
> > +#define PCIMCR_MRSET 0x40000000
> > +#define PCIMCR_RFSH  0x00000004
> > +
> > +/* board depend PCI bus fixups */
> > +static void __init julian_fixup(void __iomem *pci_reg_base, void __iomem *bcr)
> 
> Please drop all the __init* annotations.
> Although I no longer see invalid section warnings, all symbols tagged
> with __init* are still referenced from sh7751_pci_probe(), eventually.
> 
> > +{
> > +       unsigned long bcr1, mcr;
> > +
> > +       bcr1 = __raw_readl(bcr + SH7751_BCR1);
> > +       bcr1 |= 0x00080000;     /* Enable Bit 19 BREQEN, set PCIC to slave */
> > +       pcic_writel(bcr1, SH4_PCIBCR1);
> > +
> > +       mcr = __raw_readl(bcr + SH7751_MCR);
> > +       mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
> > +       pcic_writel(mcr, SH4_PCIMCR);
> > +
> > +       pcic_writel(0x0c000000, SH7751_PCICONF5);
> > +       pcic_writel(0xd0000000, SH7751_PCICONF6);
> > +       pcic_writel(0x0c000000, SH4_PCILAR0);
> > +       pcic_writel(0x00000000, SH4_PCILAR1);
> > +}
> > +
> > +static void __init r2d_fixup(void __iomem *pci_reg_base, void __iomem *bcr)
> > +{
> > +       unsigned long bcr1, mcr;
> > +
> > +       bcr1 = ioread32(bcr + SH7751_BCR1);
> > +       bcr1 |= 0x40080000;     /* Enable Bit 19 BREQEN, set PCIC to slave */
> > +       pcic_writel(bcr1, SH4_PCIBCR1);
> > +
> > +       /* Enable all interrupts, so we known what to fix */
> > +       pcic_writel(0x0000c3ff, SH4_PCIINTM);
> > +       pcic_writel(0x0000380f, SH4_PCIAINTM);
> > +
> > +       pcic_writel(0xfb900047, SH7751_PCICONF1);
> > +       pcic_writel(0xab000001, SH7751_PCICONF4);
> > +
> > +       mcr = ioread32(bcr + SH7751_MCR);
> > +       mcr &= (~PCIMCR_MRSET) & (~PCIMCR_RFSH);
> > +       pcic_writel(mcr, SH4_PCIMCR);
> > +
> > +       pcic_writel(0x0c000000, SH7751_PCICONF5);
> > +       pcic_writel(0xd0000000, SH7751_PCICONF6);
> > +       pcic_writel(0x0c000000, SH4_PCILAR0);
> > +       pcic_writel(0x00000000, SH4_PCILAR1);
> > +}
> > +
> > +static const __initconst struct fixups {
> > +       char *compatible;
> > +       void (*fixup)(void __iomem *pci_reg_base, void __iomem *bcr);
> > +} fixup_list[] = {
> > +       {
> > +               .compatible = "iodata,julian-pci",
> > +               .fixup = julian_fixup,
> > +       },
> > +       {
> > +               .compatible = "renesas,r2d-pci",
> > +               .fixup = r2d_fixup,
> > +       },
> > +};
> 
> These fixups seem to be board-specific instead of specific to the
> PCI block in the SoCs on these boards.
> 
> I see three options to handle this in a more appropriate way:
>   1. Handle this in the bootloader.
>      Not an attractive solution, as not everyone can/wants to update
>      the bootloader,
>   2. Use of_machine_is_compatible() in a platform-specific quirk
>      handler, outside the PCI driver,
>   3. Move the common parts into sh7751_pci_probe(), and the
>      handle the differences through DT topology analysis and/or
>      properties in DT.

I think the bootloader is not initialized on targets that do not use
a PCI device for booting.
I think it's better to use option 2 or 3.
I looked at the current fixup, but the only difference is the PCIC setting,
so I will try plan 3.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Yosinori Sato

Re: [RFC PATCH v2 07/30] drivers/pci: SH7751 PCI Host bridge controller driver.

[Bjorn Helgaas]

On Wed, Sep 13, 2023 at 06:23:32PM +0900, Yoshinori Sato wrote:

See subject line comments at
https://lore.kernel.org/r/20230918191602.GA201859@bhelgaas

Please add a commit log.  Repeating the subject line is fine, but
there's a little more detail that could be included here, e.g., a hint
about what platforms this is useful for.

> + * SH7751 PCI driver
> + * Copyright (C) 2023 Yoshinori Sato
> + *

Spurious blank line.

> +#define pcic_writel(val, reg) __raw_writel(val, pci_reg_base + (reg))
> +#define pcic_readl(reg) __raw_readl(pci_reg_base + (reg))

Best to include a pointer to a struct in the macro arguments so this
doesn't depend on local variable names in the users.  See advk_readl()
and advk_writel(), for example.

> +DEFINE_RAW_SPINLOCK(pci_config_lock);

Should be static.

> +	pcic_writel(0x0c000000, SH7751_PCICONF5);
> +	pcic_writel(0xd0000000, SH7751_PCICONF6);
> +	pcic_writel(0x0c000000, SH4_PCILAR0);
> +	pcic_writel(0x00000000, SH4_PCILAR1);

The header file uses upper-case hex, but this file looks like mostly
lower-case.  Maybe make them consistent?

> + * Since SH4 only does 32bit access we'll have to do a read,
> + * mask,write operation.

This RMW corrupts some registers; see comments and warning in
pci_generic_config_write32().  A comment here is probably enough.

> + * We'll allow an odd byte offset, though it should be illegal.
> + */
> +static int sh4_pci_write(struct pci_bus *bus, unsigned int devfn,
> +			 int where, int size, u32 val)

> +static int area_sdram_check(void __iomem *pci_reg_base,
> +			    void __iomem *bcr,
> +			    unsigned int area)
> +{
> +	unsigned long word;
> +
> +	word = __raw_readl(bcr + SH7751_BCR1);
> +	/* check BCR for SDRAM in area */
> +	if (((word >> area) & 1) == 0) {
> +		pr_info("PCI: Area %d is not configured for SDRAM. BCR1=0x%lx\n",

Use dev_info().  Will require passing a controller struct around,
which is common style in drivers/pci/controller/.

> +	dev_info(&pdev->dev, "PCI core found at %p\n",
> +		pci_reg_base);

Use %pR.

> +	/* use round robin mode to stop a device starving/overruning */

overrunning

Bjorn

[RFC PATCH v2 08/30] drivers/pci: Add SH7751 Host bridge controller

[Yoshinori Sato]

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 drivers/pci/controller/Kconfig  | 9 +++++++++
 drivers/pci/controller/Makefile | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index c0c3f2824990..037ff44bd1e8 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -342,6 +342,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 37c8663de7fe..ce96cfe9f1f2 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -39,6 +39,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/
-- 
2.39.2

Re: [RFC PATCH v2 08/30] drivers/pci: Add SH7751 Host bridge controller

[Geert Uytterhoeven]

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for your patch!

> ---
>  drivers/pci/controller/Kconfig  | 9 +++++++++
>  drivers/pci/controller/Makefile | 1 +

Please combine this with "[RFC PATCH v2 07/30] drivers/pci: SH7751
PCI Host bridge controller driver.".

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH v2 08/30] drivers/pci: Add SH7751 Host bridge controller

[Bjorn Helgaas]

On Wed, Sep 13, 2023 at 06:23:33PM +0900, Yoshinori Sato wrote:

Update subject line and include a commit log.

> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  drivers/pci/controller/Kconfig  | 9 +++++++++
>  drivers/pci/controller/Makefile | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index c0c3f2824990..037ff44bd1e8 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -342,6 +342,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.

Sort this so it appears in alpha order by vendor, device in
menuconfig, etc.

Since I didn't get the entire series, here are the whitespace errors
from git-am:

  Applying: arch/sh: head_32.S passing FDT address to initialize function.
  .git/rebase-apply/patch:25: trailing whitespace.
  Applying: Documentation/devicetree: Add renesas,sh7751-cpg binding document.
  .git/rebase-apply/patch:66: space before tab in indent.
		  clocks = <&xtal>;
  Applying: drivers/irqchip: SH7751 IRL external encoder with enable gate.
  .git/rebase-apply/patch:33: new blank line at EOF.

Re: [RFC PATCH v2 08/30] drivers/pci: Add SH7751 Host bridge controller

[Geert Uytterhoeven]

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -342,6 +342,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.
> +

When compile-testing:

    drivers/pci/controller/pci-sh7751.c: In function ‘sh7751_pci_probe’:
    drivers/pci/controller/pci-sh7751.c:276:16: error: implicit
declaration of function ‘P2SEGADDR’
[-Werror=implicit-function-declaration]
      276 |         word = P2SEGADDR(wres->start);
          |                ^~~~~~~~~

P2SEGADDR() depends on the SH-specific CONFIG_29BIT.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[RFC PATCH v2 09/30] Documentation/devicetree: Add renesas,sh7751-pci binding document.

[Yoshinori Sato]

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 .../bindings/pci/renesas,sh7751-pci.yaml      | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml

diff --git a/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
new file mode 100644
index 000000000000..17a24e24c393
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/renesas,sh7751-pci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas SH7751 PCI Host controller
+
+maintainers:
+  - Yoshinori Sato <ysato@users.sourceforge.jp>
+
+properties:
+  compatible:
+      - items:
+          - enum:
+              - renesas,r2d-pci	         # Renesas RTS7751R2D board
+              - iodata,julian-pci        # IO DATA DEVICE Julian board
+          - const: renesas,sh7751-pci
+
+  reg:
+    minItems: 3
+
+  interrupt-cells:
+    const: 1
+
+  address-cells:
+    const: 3
+
+  size-cells:
+    const: 2;
+
+  range:
+    description: |
+      The PCI bus memory area and I/O area.
+
+
+required:
+  - compatible
+  - reg
+  - interrupt-cells
+  - address-cells
+  - size-cells
+  - range
+
+examples:
+  - |
+        pci@fe200000 {
+                compatible = "renesas,sh7751-pci";
+                #address-cells = <3>;
+                #size-cells = <2>;
+                ranges = <0x02000000 0 0xfd000000 0xfd000000 0 0x01000000>,
+                         <0x01000000 0 0xfe240000 0xfe240000 0 0x00040000>;
+                reg = <0xfe200000 0x0400>,
+                      <0x0c000000 0x04000000>,
+                      <0xff800000 0x0030>;
+                #interrupt-cells = <1>;
+        };
-- 
2.39.2

Re: [RFC PATCH v2 09/30] Documentation/devicetree: Add renesas,sh7751-pci binding document.

[Krzysztof Kozlowski]

On 13/09/2023 11:23, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

You did not resolve several comments from previous version. I don't
understand why, so I just assume this is not ready for review. Just
quick look tells this wasn't tested and has multiple issues. Maybe that
was the intention, but nothing is described in commit log confirming
such intention.

Therefore to be clear: that's a NAK.

Also, standard template:

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.


Best regards,
Krzysztof

Re: [RFC PATCH v2 09/30] Documentation/devicetree: Add renesas,sh7751-pci binding document.

[Geert Uytterhoeven]

Hi Sato-san,

On Wed, Sep 13, 2023 at 11:24 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/renesas,sh7751-pci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7751 PCI Host controller
> +
> +maintainers:
> +  - Yoshinori Sato <ysato@users.sourceforge.jp>
> +
> +properties:
> +  compatible:
> +      - items:

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:14:7:
[warning] wrong indentation: expected 4 but found 6 (indentation)

> +          - enum:
> +              - renesas,r2d-pci                 # Renesas RTS7751R2D board

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml:16:32:
[error] syntax error: found character '\t' that cannot start any token
(syntax)

> +              - iodata,julian-pci        # IO DATA DEVICE Julian board

Please drop the two board-specific compatible values, they do not
represent different implementations of the PCI core in the SH7751 SoC.

> +          - const: renesas,sh7751-pci
> +
> +  reg:
> +    minItems: 3
> +
> +  interrupt-cells:
> +    const: 1
> +
> +  address-cells:
> +    const: 3
> +
> +  size-cells:
> +    const: 2;
> +
> +  range:
> +    description: |
> +      The PCI bus memory area and I/O area.
> +
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-cells
> +  - address-cells
> +  - size-cells
> +  - range
> +
> +examples:
> +  - |
> +        pci@fe200000 {

Please align "pci" with the "|" above

> +                compatible = "renesas,sh7751-pci";

Please indent by 4 spaces

> +                #address-cells = <3>;
> +                #size-cells = <2>;
> +                ranges = <0x02000000 0 0xfd000000 0xfd000000 0 0x01000000>,
> +                         <0x01000000 0 0xfe240000 0xfe240000 0 0x00040000>;
> +                reg = <0xfe200000 0x0400>,
> +                      <0x0c000000 0x04000000>,
> +                      <0xff800000 0x0030>;
> +                #interrupt-cells = <1>;

    Documentation/devicetree/bindings/pci/renesas,sh7751-pci.example.dtb:
pci@fe200000: 'device_type' is a required property
        from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#

> +        };

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds