Add latest U-boot support for BPI R2 & BPI R64 (not yet)

Ryder.Lee · July 30, 2019, 3:15am

ryder sent the Patches upstream, if they got accepted can you post your Patches too?

From what i see, the current PCIe driver is a intermediate version of upstream patch and it should switch to use the vanilla kernel’s one …

frank-w · July 30, 2019, 9:19am

@ray: with the help of @ryder.lee i got the pcie-node converted to 32bit so we don’t need to change full dtsi-file to 64bit

i also used Patches from ryder (posted to mailing-list) as base and put your Patches on top (without hifsys-dts-node)

now we can focus on pcie-driver to be compatible with other soc using mainline-linux-driver

ray · July 30, 2019, 9:24am

Sorry @frank-w, but it a bit silly U-Boot may use FDT, may not. But it must supply full working board FDT to system. Just like PC BIOS supply ACPI info to OS. So better to include latest full DTS and make U-Boot to work with it.

frank-w · July 30, 2019, 9:43am

imho we should only touch nodes needed to get pcie working and not convert all existing nodes to 64

do you need 64bit-nodes for EFI?

i don’t know how your EFI/Freebsd is working…but for linux there will passed the dts from linux-kernel (built-in or separately) to the system…so for this we don’t need the complete dts in uboot

ray · July 30, 2019, 10:27am

Linux just hardcode everything

FreeBSD has GENERIC kernel for AArch32, which cover many devices. Not all, but many.

We have option to make world better And don’t make private kernel for each device.

So I vote for full, recent FDT tree.

frank-w · July 30, 2019, 11:20am

we should add additional content later…

i’ve looked in linux pcie-driver (https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-mediatek.c) which is much bigger than your current version (and there is currently no mtk-pcie-driver in uboot)…so i think we also should add your version first and extend it later

i try to prepare my/your last commits for posting to mailinglist, ok?

igorpec · July 30, 2019, 11:21am

All development kernels are private ones. Once things are sorted out, when things works, they can become a part of the GENERIC Linux kernel. Not before.

ray · July 30, 2019, 11:27am

Yup. Many thanks to you!

ray · July 30, 2019, 11:29am

There is nothing generic in your GENERIC, @igorpec

Generic - mean something used by many.

In our case: by many devices.

frank-w · July 30, 2019, 11:43am

https://github.com/frank-w/u-boot/commits/2019-07-bpi-r2-sata_new

will this be ok (patches 2-5)? i have “owned” the dts-patch as i did the 32bit-translation, in the others you are the author and i added subject-tagging and our signed-off & my tested

if you ack, i do the checkpatch and send it out if no problems found

ray · July 30, 2019, 12:00pm

If you don’t like idea to include whole DTS, then I just agree.

But better to copy linux one with set of required “dt-binding” and fix some drivers to work with it.

frank-w · July 30, 2019, 12:07pm

for now we add pci-e to get ahci working in upstream for this specific device

later the full dts can be added, but currently this is not related to the functionality we want to add. when sending Patches it should not mixed up with other changes…we want to add ahci, which is not related to most of the other dts-nodes

also the pci-e-driver can be updated to newer version or extended afterwards for other devices. for now i can only test mt7623/bpi-r2 and so i want to send it. if anyone wants to add another device/soc (which he can test it) the driver can be changed/maybe renamed

i agree with ryder make pcie-driver universal to supporting other devices too, but i have not the knowledge and cannot test on other devices yet so this is a change for later Patchset

FYI, checkpatch-result, i try to fix now

Zusammenfassung

[13:54:36]$ scripts/checkpatch.pl 0001-ahci-pci-ASM1061-report-wrong-class-but-support-AHCI.patch
total: 0 errors, 0 warnings, 0 checks, 7 lines checked

0001-ahci-pci-ASM1061-report-wrong-class-but-support-AHCI.patch has no obvious style problems and is ready for submission.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE

[13:54:57]$ scripts/checkpatch.pl 0002-ata-ahci-Don-t-forget-to-clear-upper-address-regs.patch 
CHECK: Please don't use multiple blank lines
#23: FILE: drivers/ata/ahci.c:596:
 
+

total: 0 errors, 0 warnings, 1 checks, 18 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

0002-ata-ahci-Don-t-forget-to-clear-upper-address-regs.patch has style problems, please review.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[13:55:09]$ scripts/checkpatch.pl 0003-pci-mediatek-Add-pci-driver-for-mt2701.patch 
WARNING: please write a paragraph that describes the config symbol fully
#26: FILE: drivers/pci/Kconfig:148:
+config PCIE_MT2701

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#47: FILE: drivers/pci/pci-mt2701.c:1:
+/*

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#71: FILE: drivers/pci/pci-mt2701.c:25:
+#define iowrite32(v, a)	((*((volatile u32 *)(a))) = v)

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#72: FILE: drivers/pci/pci-mt2701.c:26:
+#define iowrite16(v, a)	((*((volatile u16 *)(a))) = v)

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#73: FILE: drivers/pci/pci-mt2701.c:27:
+#define iowrite8(v, a)	((*((volatile u8 *)(a))) = v)

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#74: FILE: drivers/pci/pci-mt2701.c:28:
+#define ioread32(a)	((*((volatile u32 *)(a))))

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#75: FILE: drivers/pci/pci-mt2701.c:29:
+#define ioread16(a)	((*((volatile u16 *)(a))))

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#76: FILE: drivers/pci/pci-mt2701.c:30:
+#define ioread8(a)	((*((volatile u8 *)(a))))

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#85: FILE: drivers/pci/pci-mt2701.c:39:
+#define RD(x)		(*((volatile u32 *)(RT_PCIE_BASE | (x))))

WARNING: please, no space before tabs
#86: FILE: drivers/pci/pci-mt2701.c:40:
+#define WR(x, v) ^I(*((volatile u32 *)(RT_PCIE_BASE | (x))) = (v))$

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#86: FILE: drivers/pci/pci-mt2701.c:40:
+#define WR(x, v) 	(*((volatile u32 *)(RT_PCIE_BASE | (x))) = (v))

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#164: FILE: drivers/pci/pci-mt2701.c:118:
+	uint32_t reg;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#165: FILE: drivers/pci/pci-mt2701.c:119:
+	uint32_t mask;

CHECK: Prefer kernel type 'u32' over 'uint32_t'
#166: FILE: drivers/pci/pci-mt2701.c:120:
+	uint32_t val;

WARNING: externs should be avoided in .c files
#189: FILE: drivers/pci/pci-mt2701.c:143:
+int mtk_pcie_probe(void);

CHECK: Alignment should match open parenthesis
#191: FILE: drivers/pci/pci-mt2701.c:145:
+static int mt_pcie_read_config(struct udevice *, pci_dev_t, uint, ulong *,
+		enum pci_size_t);

ERROR: space prohibited before that ',' (ctx:WxE)
#192: FILE: drivers/pci/pci-mt2701.c:146:
+static int mt_pcie_write_config(struct udevice *, pci_dev_t, uint, ulong ,
                                                                          ^

CHECK: Alignment should match open parenthesis
#193: FILE: drivers/pci/pci-mt2701.c:147:
+static int mt_pcie_write_config(struct udevice *, pci_dev_t, uint, ulong ,
+		enum pci_size_t);

CHECK: Macro argument reuse 'p' - possible side-effects?
#195: FILE: drivers/pci/pci-mt2701.c:149:
+#define mtk_foreach_port(p)		\
+		for ((p) = mtk_pcie_port;	\
+		(p) != &mtk_pcie_port[ARRAY_SIZE(mtk_pcie_port)]; (p)++)

ERROR: space required after that ',' (ctx:VxV)
#198: FILE: drivers/pci/pci-mt2701.c:152:
+#define BITS(m,n)   (~(BIT(m)-1) & ((BIT(n) - 1) | BIT(n)))
               ^

CHECK: spaces preferred around that '-' (ctx:VxV)
#198: FILE: drivers/pci/pci-mt2701.c:152:
+#define BITS(m,n)   (~(BIT(m)-1) & ((BIT(n) - 1) | BIT(n)))
                              ^

CHECK: Macro argument reuse 'n' - possible side-effects?
#198: FILE: drivers/pci/pci-mt2701.c:152:
+#define BITS(m,n)   (~(BIT(m)-1) & ((BIT(n) - 1) | BIT(n)))

ERROR: trailing whitespace
#204: FILE: drivers/pci/pci-mt2701.c:158:
+^I$

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#206: FILE: drivers/pci/pci-mt2701.c:160:
+	regValue = le32_to_cpu(*(volatile u_long *)(0x100059f0));

ERROR: space required after that ',' (ctx:VxV)
#207: FILE: drivers/pci/pci-mt2701.c:161:
+	regValue &= ~(BITS(9,11));
 	                    ^

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#209: FILE: drivers/pci/pci-mt2701.c:163:
+	*(volatile u_long *)(0x100059f0) = regValue;

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#212: FILE: drivers/pci/pci-mt2701.c:166:
+	regValue = le32_to_cpu(*(volatile u_long *)(0x100059f0));

ERROR: space required after that ',' (ctx:VxV)
#213: FILE: drivers/pci/pci-mt2701.c:167:
+	regValue &= ~(BITS(12,14));
 	                     ^

WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
#215: FILE: drivers/pci/pci-mt2701.c:169:
+	*(volatile u_long *)(0x100059f0) = regValue;

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#219: FILE: drivers/pci/pci-mt2701.c:173:
+sys_w32(struct mtk_pcie *pcie, u32 val, unsigned reg)

ERROR: trailing whitespace
#221: FILE: drivers/pci/pci-mt2701.c:175:
+^I$

CHECK: Blank lines aren't necessary after an open brace '{'
#221: FILE: drivers/pci/pci-mt2701.c:175:
+{
+	

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#226: FILE: drivers/pci/pci-mt2701.c:180:
+sys_r32(struct mtk_pcie *pcie, unsigned reg)

ERROR: trailing whitespace
#228: FILE: drivers/pci/pci-mt2701.c:182:
+^I$

CHECK: Blank lines aren't necessary after an open brace '{'
#228: FILE: drivers/pci/pci-mt2701.c:182:
+{
+	

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#233: FILE: drivers/pci/pci-mt2701.c:187:
+pcie_w32(struct mtk_pcie *pcie, u32 val, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#235: FILE: drivers/pci/pci-mt2701.c:189:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#240: FILE: drivers/pci/pci-mt2701.c:194:
+pcie_w16(struct mtk_pcie *pcie, u16 val, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#242: FILE: drivers/pci/pci-mt2701.c:196:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#247: FILE: drivers/pci/pci-mt2701.c:201:
+pcie_w8(struct mtk_pcie *pcie, u8 val, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#249: FILE: drivers/pci/pci-mt2701.c:203:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#254: FILE: drivers/pci/pci-mt2701.c:208:
+pcie_r32(struct mtk_pcie *pcie, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#256: FILE: drivers/pci/pci-mt2701.c:210:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#261: FILE: drivers/pci/pci-mt2701.c:215:
+pcie_r16(struct mtk_pcie *pcie, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#263: FILE: drivers/pci/pci-mt2701.c:217:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#268: FILE: drivers/pci/pci-mt2701.c:222:
+pcie_r8(struct mtk_pcie *pcie, unsigned reg)

CHECK: Blank lines aren't necessary after an open brace '{'
#270: FILE: drivers/pci/pci-mt2701.c:224:
+{
+

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#275: FILE: drivers/pci/pci-mt2701.c:229:
+pcie_m32(struct mtk_pcie *pcie, u32 mask, u32 val, unsigned reg)

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#300: FILE: drivers/pci/pci-mt2701.c:254:
+	udelay(16000);

WARNING: line over 80 characters
#308: FILE: drivers/pci/pci-mt2701.c:262:
+	mt_pcie_write_config(NULL, (port->id)<<3, PCI_BASE_ADDRESS_0, 0x80000000,

CHECK: spaces preferred around that '<<' (ctx:VxV)
#308: FILE: drivers/pci/pci-mt2701.c:262:
+	mt_pcie_write_config(NULL, (port->id)<<3, PCI_BASE_ADDRESS_0, 0x80000000,
 	                                     ^

CHECK: Alignment should match open parenthesis
#309: FILE: drivers/pci/pci-mt2701.c:263:
+	mt_pcie_write_config(NULL, (port->id)<<3, PCI_BASE_ADDRESS_0, 0x80000000,
+			PCI_SIZE_32);

CHECK: spaces preferred around that '<<' (ctx:VxV)
#310: FILE: drivers/pci/pci-mt2701.c:264:
+	mt_pcie_read_config(NULL, (port->id)<<3, PCI_BASE_ADDRESS_0, &val,
 	                                    ^

CHECK: Alignment should match open parenthesis
#311: FILE: drivers/pci/pci-mt2701.c:265:
+	mt_pcie_read_config(NULL, (port->id)<<3, PCI_BASE_ADDRESS_0, &val,
+			PCI_SIZE_32);

CHECK: spaces preferred around that '<<' (ctx:VxV)
#315: FILE: drivers/pci/pci-mt2701.c:269:
+	mt_pcie_read_config(NULL, (port->id)<<3, 0x73c, &val, PCI_SIZE_32);
 	                                    ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#316: FILE: drivers/pci/pci-mt2701.c:270:
+	val &= ~(0x9fffUL)<<16;
 	                  ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#317: FILE: drivers/pci/pci-mt2701.c:271:
+	val |= 0x806c<<16;
 	             ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#318: FILE: drivers/pci/pci-mt2701.c:272:
+	mt_pcie_write_config(NULL, (port->id)<<3, 0x73c, val, PCI_SIZE_32);
 	                                     ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#319: FILE: drivers/pci/pci-mt2701.c:273:
+	mt_pcie_read_config(NULL, (port->id)<<3, 0x73c, &val, PCI_SIZE_32);
 	                                    ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#322: FILE: drivers/pci/pci-mt2701.c:276:
+	mt_pcie_read_config(NULL, (port->id)<<3, 0x70c, &val, PCI_SIZE_32);
 	                                    ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#325: FILE: drivers/pci/pci-mt2701.c:279:
+	mt_pcie_write_config(NULL, (port->id)<<3, 0x70c, val, PCI_SIZE_32);
 	                                     ^

CHECK: spaces preferred around that '<<' (ctx:VxV)
#326: FILE: drivers/pci/pci-mt2701.c:280:
+	mt_pcie_read_config(NULL, (port->id)<<3, 0x70c, &val, PCI_SIZE_32);
 	                                    ^

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#344: FILE: drivers/pci/pci-mt2701.c:298:
+	udelay(12000);

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#346: FILE: drivers/pci/pci-mt2701.c:300:
+	udelay(12000);

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#367: FILE: drivers/pci/pci-mt2701.c:321:
+	udelay(12000);

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#369: FILE: drivers/pci/pci-mt2701.c:323:
+	udelay(200000);

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#382: FILE: drivers/pci/pci-mt2701.c:336:
+	udelay(200000);

WARNING: braces {} are not necessary for single statement blocks
#392: FILE: drivers/pci/pci-mt2701.c:346:
+		if (port->link) {
+			pcie->pcie_card_link++;
+		}

WARNING: long udelay - prefer mdelay; see arch/arm/include/asm/delay.h
#411: FILE: drivers/pci/pci-mt2701.c:365:
+	udelay(100000);

CHECK: if this code is redundant consider removing it
#421: FILE: drivers/pci/pci-mt2701.c:375:
+#if 0

CHECK: Alignment should match open parenthesis
#428: FILE: drivers/pci/pci-mt2701.c:382:
+		printf("%s: PCIe/USB3 combo PHY mode SYSCFG1=%x\n",
+			__func__, val);

CHECK: Alignment should match open parenthesis
#432: FILE: drivers/pci/pci-mt2701.c:386:
+		printf("%s: PCIe/USB3 combo PHY mode SYSCFG1=%x\n",
+			__func__, val);

WARNING: line over 80 characters
#458: FILE: drivers/pci/pci-mt2701.c:412:
+	pci_set_region(&hose->regions[0], RT_PCIE_IOWIN_BASE, RT_PCIE_IOWIN_BASE,

WARNING: line over 80 characters
#462: FILE: drivers/pci/pci-mt2701.c:416:
+	pci_set_region(&hose->regions[1], RT_PCIE_MEMWIN_BASE, RT_PCIE_MEMWIN_BASE,

CHECK: Please don't use multiple blank lines
#468: FILE: drivers/pci/pci-mt2701.c:422:
+
+

CHECK: Alignment should match open parenthesis
#497: FILE: drivers/pci/pci-mt2701.c:451:
+mt_pcie_read_config(struct udevice *bus, pci_dev_t bdf,
+			       uint offset, ulong *valuep,

WARNING: Missing a blank line after declarations
#501: FILE: drivers/pci/pci-mt2701.c:455:
+	u32 address =  bdf | ((offset & 0xf00) << 16) | (offset & 0xfc);
+	pcie_m32(pcie0, 0xf0000000, address, CFGADDR);

CHECK: Alignment should match open parenthesis
#520: FILE: drivers/pci/pci-mt2701.c:474:
+mt_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
+				uint offset, ulong value,

WARNING: Missing a blank line after declarations
#524: FILE: drivers/pci/pci-mt2701.c:478:
+	u32 address =  bdf | ((offset & 0xf00) << 16) | (offset & 0xfc);
+	pcie_m32(pcie0, 0xf0000000, address, CFGADDR);

CHECK: Please don't use multiple blank lines
#549: FILE: drivers/pci/pci-mt2701.c:503:
+
+

total: 7 errors, 39 warnings, 35 checks, 534 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

0003-pci-mediatek-Add-pci-driver-for-mt2701.patch has style problems, please review.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

[13:55:39]$ scripts/checkpatch.pl 0004-arm-dts-Add-PCI-E-controller-for-mt7623.patch 
WARNING: line over 80 characters
#34: FILE: arch/arm/dts/mt7623.dtsi:270:
+		interrupt-map = <0x0000 0 0 0 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>,

WARNING: line over 80 characters
#35: FILE: arch/arm/dts/mt7623.dtsi:271:
+				<0x0800 0 0 0 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>,

WARNING: line over 80 characters
#36: FILE: arch/arm/dts/mt7623.dtsi:272:
+				<0x1000 0 0 0 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;

WARNING: line over 80 characters
#63: FILE: arch/arm/dts/mt7623.dtsi:299:
+			interrupt-map = <0 0 0 0 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>;

WARNING: line over 80 characters
#76: FILE: arch/arm/dts/mt7623.dtsi:312:
+			interrupt-map = <0 0 0 0 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>;

WARNING: line over 80 characters
#89: FILE: arch/arm/dts/mt7623.dtsi:325:
+			interrupt-map = <0 0 0 0 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;

total: 0 errors, 6 warnings, 0 checks, 114 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

0004-arm-dts-Add-PCI-E-controller-for-mt7623.patch has style problems, please review.

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

@ray how can i fix the messages:

WARNING: Use of volatile is usually wrong => how to define the ioread/write another way without volatile? below there also calls with volatile as far as i see it’s always “*(volatile u_long *)(0x100059f0)”, imho we can use ioread32/iowrite32 here because there is a 32bit-pointer passed
WARNING: line over 80 characters in DTS?

please check the other fixes if they are right…have pushed as separate commit, will be squashed later

igorpec · July 30, 2019, 3:49pm

Mainline kernel (www.kernel.org) is generic and used by many devices.

frank-w · July 30, 2019, 3:50pm

Ray wants to use freebsd not linux kernel…

But now please back 2 topic. Can you give me an advice how to fix the other warnings?

ray · July 30, 2019, 5:45pm

You can replace it with:

val32 = readl(val, base + reg);
writel(val32, base + reg);

Break that line:

<0x1000 0 0 0 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>;

as:

<0x1000 0 0 0 &sysirq GIC_SPI 195
    IRQ_TYPE_LEVEL_LOW>;

should be fine.

frank-w · July 30, 2019, 5:52pm

what is base and reg? i guess 0x100059f0 is both, right? i also guess readl does not have a val…

can we also replace the 8bit/16bit-variants?

https://elixir.bootlin.com/u-boot/latest/source/arch/arm/include/asm/io.h#L122

seems there is also readb (byte=8bit) and readw (word=16bit)

are my current fixes right? have now uploaded the volatile fixes. Basicly i can drop ioread/write completely and use read/write direct…

ray · July 30, 2019, 8:30pm

Yup. In that case 0x10005000 is base, 0x09f0 is reg.

Sure.

Can you fix discussed + maybe that I commented in your repo and them drop me a link to final review?

frank-w · July 30, 2019, 8:33pm

Is usage of readl/writel right and the fixes before?

Is there any name (to use as constant) for 0x09f0? Maybe also for “base”

ray · July 30, 2019, 9:29pm

Usage like that, correct:

val32 = readl(base + reg);
writel(val32, base + reg);

Other fixes looks good.

Base value you can name as PCIE_PHY_BASE, but 0x09f0 goes from old PCIE driver of MIPS Mediatek SoC. And there is no names

But it is possible to get them from recent Linux kernel tree. Seems from its PHY driver.

frank-w · July 31, 2019, 6:12am

Maybe some of them:

github.com

torvalds/linux/blob/master/drivers/phy/mediatek/phy-mtk-tphy.c

// SPDX-License-Identifier: GPL-2.0
/*
 * Copyright (c) 2015 MediaTek Inc.
 * Author: Chunfeng Yun <[email protected]>
 *
 */

#include <dt-bindings/phy/phy.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>

/* version V1 sub-banks offset base address */
/* banks shared by multiple phys */

This file has been truncated. show original

But first i test and squash current changes