From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 476D2CE79CE for ; Wed, 20 Sep 2023 12:16:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235489AbjITMQ3 convert rfc822-to-8bit (ORCPT ); Wed, 20 Sep 2023 08:16:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235544AbjITMQV (ORCPT ); Wed, 20 Sep 2023 08:16:21 -0400 Received: from hsmtpd-def.xspmail.jp (hsmtpd-def.xspmail.jp [202.238.198.244]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D09CCC1 for ; Wed, 20 Sep 2023 05:16:04 -0700 (PDT) X-Country-Code: JP Received: from sakura.ysato.name (ik1-413-38519.vs.sakura.ne.jp [153.127.30.23]) by hsmtpd-out-2.asahinet.cluster.xspmail.jp (Halon) with ESMTPA id 11278766-12f6-4d70-ad13-d88e0ea51358; Wed, 20 Sep 2023 21:16:02 +0900 (JST) Received: from SIOS1075.ysato.ml (al128006.dynamic.ppp.asahi-net.or.jp [111.234.128.6]) by sakura.ysato.name (Postfix) with ESMTPSA id 6F1FD1C00DC; Wed, 20 Sep 2023 21:16:00 +0900 (JST) Date: Wed, 20 Sep 2023 21:15:59 +0900 Message-ID: <87sf79t4zk.wl-ysato@users.sourceforge.jp> From: Yoshinori Sato To: Geert Uytterhoeven Cc: linux-sh@vger.kernel.org, glaubitz@physik.fu-berlin.de, linux-pci@vger.kernel.org Subject: Re: [RFC PATCH v2 07/30] drivers/pci: SH7751 PCI Host bridge controller driver. In-Reply-To: References: <7f25af9e93fbb84c8e4fe6da3c0c13b0a6be2c73.1694596125.git.ysato@users.sourceforge.jp> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org 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 > wrote: > > Signed-off-by: Yoshinori Sato > > 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 > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#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