* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Duc Dang @ 2016-09-12 22:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADaLNDk0JPJYT_pqjWYwrMBvbyWkpL+6=dEDU9H9qDGMhnQvyw@mail.gmail.com>
On Mon, Sep 12, 2016 at 3:24 PM, Duc Dang <dhdang@apm.com> wrote:
> On Fri, Sep 9, 2016 at 12:24 PM, Tomasz Nowicki <tn@semihalf.com> wrote:
>>
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite CFG accessors
>> set and configuration space range.
>>
>> In first place pci_mcfg_parse() saves machine's IDs and revision number
>> (these come from MCFG header) in order to match against known quirk entries.
>> Then the algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns custom PCI config ops and/or CFG resource structure.
>>
>> When adding new quirk there are two possibilities:
>> 1. Override default pci_generic_ecam_ops ops but CFG resource comes from MCFG
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, MCFG_RES_EMPTY },
>> 2. Override default pci_generic_ecam_ops ops and CFG resource. For this case
>> it is also allowed get CFG resource from quirk entry w/o having it in MCFG.
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops,
>> DEFINE_RES_MEM(START, SIZE) },
>>
>> pci_generic_ecam_ops and MCFG entries will be used for platforms
>> free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>> drivers/acpi/pci_mcfg.c | 80 +++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 74 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index ffcc651..2b8acc7 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -32,6 +32,59 @@ struct mcfg_entry {
>> u8 bus_start;
>> u8 bus_end;
>> };
>> +struct mcfg_fixup {
>> + char oem_id[ACPI_OEM_ID_SIZE + 1];
>> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>> + u32 oem_revision;
>> + u16 seg;
>> + struct resource bus_range;
>> + struct pci_ecam_ops *ops;
>> + struct resource cfgres;
>> +};
>> +
>> +#define MCFG_DOM_ANY (-1)
>> +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \
>> + ((end) - (start) + 1), \
>> + NULL, IORESOURCE_BUS)
>> +#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff)
>> +#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0)
>> +
>> +static struct mcfg_fixup mcfg_quirks[] = {
>> +/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
>> +};
>> +
>> +static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> +static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
>> +static u32 mcfg_oem_revision;
>> +
>> +static void pci_mcfg_match_quirks(struct acpi_pci_root *root,
>> + struct resource *cfgres,
>> + struct pci_ecam_ops **ecam_ops)
>> +{
>> + struct mcfg_fixup *f;
>> + int i;
>> +
>> + /*
>> + * First match against PCI topology <domain:bus> then use OEM ID, OEM
>> + * table ID, and OEM revision from MCFG table standard header.
>> + */
>> + for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
>> + if (f->seg == root->segment &&
>
> Is dropping the comparison with MCFG_DOM_ANY intended? It is useful if
> all the controllers (segs) can use the same quirk (X-Gene case).
> If you decide to drop it, then we can remove MCFG_DOM_ANY definition as well.
>
>> + resource_contains(&f->bus_range, &root->secondary) &&
>> + !memcmp(f->oem_id, mcfg_oem_id, ACPI_OEM_ID_SIZE) &&
>> + !memcmp(f->oem_table_id, mcfg_oem_table_id,
>> + ACPI_OEM_TABLE_ID_SIZE) &&
>> + f->oem_revision == mcfg_oem_revision) {
>> + if (f->cfgres.start)
>> + *cfgres = f->cfgres;
>> + if (f->ops)
>> + *ecam_ops = f->ops;
>> + dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
>> + f->oem_id, f->oem_table_id, f->oem_revision);
>> + return;
>> + }
>> + }
>> +}
>>
>> /* List to save MCFG entries */
>> static LIST_HEAD(pci_mcfg_list);
>> @@ -61,14 +114,24 @@ int pci_mcfg_lookup(struct acpi_pci_root *root, struct resource *cfgres,
>>
>> }
>>
>> - if (!root->mcfg_addr)
>> - return -ENXIO;
>> -
>> skip_lookup:
>> memset(&res, 0, sizeof(res));
>> - res.start = root->mcfg_addr + (bus_res->start << 20);
>> - res.end = res.start + (resource_size(bus_res) << 20) - 1;
>> - res.flags = IORESOURCE_MEM;
>> + if (root->mcfg_addr) {
>> + res.start = root->mcfg_addr + (bus_res->start << 20);
>> + res.end = res.start + (resource_size(bus_res) << 20) - 1;
>> + res.flags = IORESOURCE_MEM;
>> + }
>> +
>> + /*
>> + * Let to override default ECAM ops and CFG resource range.
>> + * Also, this might even retrieve CFG resource range in case MCFG
>> + * does not have it. Invalid CFG start address means MCFG firmware bug
>> + * or we need another quirk in array.
>> + */
>> + pci_mcfg_match_quirks(root, &res, &ops);
>> + if (!res.start)
>> + return -ENXIO;
>> +
>> *cfgres = res;
>> *ecam_ops = ops;
>> return 0;
>> @@ -101,6 +164,11 @@ static __init int pci_mcfg_parse(struct acpi_table_header *header)
>> list_add(&e->list, &pci_mcfg_list);
>> }
>>
>> + /* Save MCFG IDs and revision for quirks matching */
>> + memcpy(mcfg_oem_id, header->oem_id, ACPI_OEM_ID_SIZE);
>> + memcpy(mcfg_oem_table_id, header->oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
>> + mcfg_oem_revision = header->revision;
I think this is a typo, it should be:
mcfg_oem_revision = header->oem_revision;
>> +
>> pr_info("MCFG table detected, %d entries\n", n);
>> return 0;
>> }
>> --
>> 1.9.1
> Regards,
> Duc Dang.
Regards,
Duc Dang.
^ permalink raw reply
* [PATCH v2 00/17] Make rpmsg a framework
From: Bjorn Andersson @ 2016-09-12 22:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912222240.GA28944@linaro.org>
On Mon 12 Sep 15:22 PDT 2016, Lina Iyer wrote:
> On Mon, Sep 12 2016 at 10:52 -0600, Lina Iyer wrote:
> >Hi Bjorn,
> >
> >On Thu, Sep 01 2016 at 16:28 -0600, Bjorn Andersson wrote:
> >>This series splits the virtio rpmsg bus driver into a rpmsg bus and a virtio
> >>backend/wireformat.
> >>
> >>
> >>As we discussed the Qualcomm SMD implementation a couple of years back people
> >>suggested that I should make it "a rpmsg thingie". With the introduction of the
> >>Qualcomm 8996 platform, we must support a variant of the communication
> >>mechanism that share many of the characteristics of SMD, but are different
> >>enough that it can't be done in a single implementation. As such there is
> >>enough benefit to do the necessary work and being able to make SMD a "rpmsg
> >>thingie".
> >>
> >>On-top of this series I have patches to switch the current smd clients over to
> >>rpmsg (and by that drop the existing SMD implementation).
> >>
> >>All this allows me to implement the new backend and reuse all existing SMD
> >>drivers with the new mechanism.
> >>
> >
> >RPM Communication has to supported even when IRQs are disabled. The most
> >important use of this communication is to set the wake up time for the
> >CPU subsystem when all the CPUs are powered off. In addition to that,
> >"sleep" votes that are sent by the application processor subsystem to
> >allow system to go into deep sleep modes can only be triggered when the
> >CPU PM domains are power collapsed, drivers do not have a knowledge of
> >when that happens. This has to be done by a platform code that registers
> >for CPU PM domain power_off/on callbacks.
> >
> Ok, my bad. These two cases are not critical for the SoC supported by
> this driver. So you are good to go from cpuidle perspective
>
Thanks for letting me know. Please keep me updated if you find any
changes to this.
Just to be clear, I'm not against supporting sending and receiving
messages in atomic context as long as it doesn't just add accidental
complexity.
Regards,
Bjorn
^ permalink raw reply
* [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Scott Wood @ 2016-09-12 23:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <HE1PR04MB08892D8354E38EC32F549E98F8FF0@HE1PR04MB0889.eurprd04.prod.outlook.com>
On Mon, 2016-09-12 at 06:39 +0000, Y.B. Lu wrote:
> Hi Scott,
>
> Thanks for your review :)
> See my comment inline.
>
> >
> > -----Original Message-----
> > From: Scott Wood [mailto:oss at buserror.net]
> > Sent: Friday, September 09, 2016 11:47 AM
> > To: Y.B. Lu; linux-mmc at vger.kernel.org; ulf.hansson at linaro.org; Arnd
> > Bergmann
> > Cc: linuxppc-dev at lists.ozlabs.org; devicetree at vger.kernel.org; linux-arm-
> > kernel at lists.infradead.org; linux-kernel at vger.kernel.org; linux-
> > clk at vger.kernel.org; linux-i2c at vger.kernel.org; iommu at lists.linux-
> > foundation.org; netdev at vger.kernel.org; Mark Rutland; Rob Herring;
> > Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> > Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie
> > Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
> >
> > On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> > >
> > > The global utilities block controls power management, I/O device
> > > enabling, power-onreset(POR) configuration monitoring, alternate
> > > function selection for multiplexed signals,and clock control.
> > >
> > > This patch adds a driver to manage and access global utilities block.
> > > Initially only reading SVR and registering soc device are supported.
> > > Other guts accesses, such as reading RCW, should eventually be moved
> > > into this driver as well.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > Signed-off-by: Scott Wood <oss@buserror.net>
> > Don't put my signoff on patches that I didn't put it on
> > myself. ?Definitely don't put mine *after* yours on patches that were
> > last modified by you.
> >
> > If you want to mention that the soc_id encoding was my suggestion, then
> > do so explicitly.
> >
> [Lu Yangbo-B47093] I found your 'signoff' on this patch at below link.
> http://patchwork.ozlabs.org/patch/649211/
>
> So, let me just change the order in next version ?
> Signed-off-by: Scott Wood <oss@buserror.net>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
No. ?This isn't my patch so my signoff shouldn't be on it.
> [Lu Yangbo-B47093] It's a good idea to move die into .family I think.
> In my opinion, it's better to keep svr and name in soc_id just like your
> suggestion above.
> >
> > {
> > .soc_id = "svr:0x85490010,name:T1023E,",
> > .family = "QorIQ T1024",
> > }
> The user probably don?t like to learn the svr value. What they want is just
> to match the soc they use.
> It's convenient to use name+rev for them to match a soc.
What the user should want 99% of the time is to match the die (plus revision),
not the soc.
> Regarding shrinking the table, I think it's hard to use svr+mask. Because I
> find many platforms use different masks.
> We couldn?t know the mask according svr value.
The mask would be part of the table:
{
{
.die = "T1024",
.svr = 0x85400000,
.mask = 0xfff00000,
},
{
.die = "T1040",
.svr = 0x85200000,
.mask = 0xfff00000,
},
{
.die = "LS1088A",
.svr = 0x87030000,
.mask = 0xffff0000,
},
...
}
There's a small risk that we get the mask wrong and a different die is created
that matches an existing table, but it doesn't seem too likely, and can easily
be fixed with a kernel update if it happens.
BTW, aren't ls2080a and ls2085a the same die? ?And is there no non-E version
of LS2080A/LS2040A?
> > > + do {
> > > + if (!matches->soc_id)
> > > + return NULL;
> > > + if (glob_match(svr_match, matches->soc_id))
> > > + break;
> > > + } while (matches++);
> > Are you expecting "matches++" to ever evaluate as false?
> [Lu Yangbo-B47093] Yes, this is used to match the soc we use in qoriq_soc
> array until getting true.?
> We need to get the name and die information defined in array.
I'm not asking whether the glob_match will ever return true. ?I'm saying that
"matches++" will never become NULL.
> > > + /* Register soc device */
> > > + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > > + if (!soc_dev_attr) {
> > > + ret = -ENOMEM;
> > > + goto out_unmap;
> > > + }
> > Couldn't this be statically allocated?
> [Lu Yangbo-B47093] Do you mean we define this struct statically ?
>
> static struct soc_device_attribute soc_dev_attr;
Yes.
> > > +
> > > + soc_dev = soc_device_register(soc_dev_attr);
> > > + if (IS_ERR(soc_dev)) {
> > > + ret = -ENODEV;
> > Why are you changing the error code?
> [Lu Yangbo-B47093] What error code should we use ? :)
ret = PTR_ERR(soc_dev);
+ }
> > > + return 0;
> > > +out:
> > > + kfree(soc_dev_attr->machine);
> > > + kfree(soc_dev_attr->family);
> > > + kfree(soc_dev_attr->soc_id);
> > > + kfree(soc_dev_attr->revision);
> > > + kfree(soc_dev_attr);
> > > +out_unmap:
> > > + iounmap(guts->regs);
> > > +out_free:
> > > + kfree(guts);
> > devm
> [Lu Yangbo-B47093] What's the devm meaning here :)
If you allocate these with devm_kzalloc(), devm_kasprintf(), devm_kstrdup(),
etc. then they will be freed automatically when the device is unbound.
> ?
> >
> >
> > >
> > > +static int fsl_guts_remove(struct platform_device *dev) {
> > > + kfree(soc_dev_attr->machine);
> > > + kfree(soc_dev_attr->family);
> > > + kfree(soc_dev_attr->soc_id);
> > > + kfree(soc_dev_attr->revision);
> > > + kfree(soc_dev_attr);
> > > + soc_device_unregister(soc_dev);
> > > + iounmap(guts->regs);
> > > + kfree(guts);
> > > + return 0;
> > > +}
> > Don't free the memory before you unregister the device that uses it (moot
> > if you use devm).
> [Lu Yangbo-B47093] The soc.c driver mentions that.
> Ensure soc_dev->attr is freed prior to calling soc_device_unregister.
That comment is wrong. ?Freeing the memory first creates a race condition that
could result in accessing freed memory, if something accesses the soc device
in parallel with unbinding.
-Scott
^ permalink raw reply
* [PATCH v4] arm64: Improve kprobes test for atomic sequence
From: Masami Hiramatsu @ 2016-09-13 0:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473704487-6069-1-git-send-email-dave.long@linaro.org>
On Mon, 12 Sep 2016 14:21:27 -0400
David Long <dave.long@linaro.org> wrote:
> From: "David A. Long" <dave.long@linaro.org>
>
> Kprobes searches backwards a finite number of instructions to determine if
> there is an attempt to probe a load/store exclusive sequence. It stops when
> it hits the maximum number of instructions or a load or store exclusive.
> However this means it can run up past the beginning of the function and
> start looking at literal constants. This has been shown to cause a false
> positive and blocks insertion of the probe. To fix this, further limit the
> backwards search to stop if it hits a symbol address from kallsyms. The
> presumption is that this is the entry point to this code (particularly for
> the common case of placing probes at the beginning of functions).
>
> This also improves efficiency by not searching code that is not part of the
> function. There may be some possibility that the label might not denote the
> entry path to the probed instruction but the likelihood seems low and this
> is just another example of how the kprobes user really needs to be
> careful about what they are doing.
>
> Signed-off-by: David A. Long <dave.long@linaro.org>
> ---
> arch/arm64/kernel/probes/decode-insn.c | 48 ++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c
> index 37e47a9..d1731bf 100644
> --- a/arch/arm64/kernel/probes/decode-insn.c
> +++ b/arch/arm64/kernel/probes/decode-insn.c
> @@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/kprobes.h>
> #include <linux/module.h>
> +#include <linux/kallsyms.h>
> #include <asm/kprobes.h>
> #include <asm/insn.h>
> #include <asm/sections.h>
> @@ -122,7 +123,7 @@ arm_probe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi)
> static bool __kprobes
> is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t *scan_end)
> {
> - while (scan_start > scan_end) {
> + while (scan_start >= scan_end) {
> /*
> * atomic region starts from exclusive load and ends with
> * exclusive store.
> @@ -142,33 +143,30 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct arch_specific_insn *asi)
> {
> enum kprobe_insn decoded;
> kprobe_opcode_t insn = le32_to_cpu(*addr);
> - kprobe_opcode_t *scan_start = addr - 1;
> - kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
> -#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
> - struct module *mod;
> -#endif
> -
> - if (addr >= (kprobe_opcode_t *)_text &&
> - scan_end < (kprobe_opcode_t *)_text)
> - scan_end = (kprobe_opcode_t *)_text;
> -#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
> - else {
> - preempt_disable();
> - mod = __module_address((unsigned long)addr);
> - if (mod && within_module_init((unsigned long)addr, mod) &&
> - !within_module_init((unsigned long)scan_end, mod))
> - scan_end = (kprobe_opcode_t *)mod->init_layout.base;
> - else if (mod && within_module_core((unsigned long)addr, mod) &&
> - !within_module_core((unsigned long)scan_end, mod))
> - scan_end = (kprobe_opcode_t *)mod->core_layout.base;
> - preempt_enable();
> + kprobe_opcode_t *scan_end = NULL;
> + unsigned long size = 0, offset = 0;
> +
> + /*
> + * If there's a symbol defined in front of and near enough to
> + * the probe address assume it is the entry point to this
> + * code and use it to further limit how far back we search
> + * when determining if we're in an atomic sequence. If we could
> + * not find any symbol skip the atomic test altogether as we
> + * could otherwise end up searching irrelevant text/literals.
> + * KPROBES depends on KALLSYMS so this last case should never
> + * happen.
> + */
> + if (kallsyms_lookup_size_offset((unsigned long) addr, &size, &offset)) {
> + if (offset < (MAX_ATOMIC_CONTEXT_SIZE*sizeof(kprobe_opcode_t)))
> + scan_end = addr - (offset / sizeof(kprobe_opcode_t));
> + else
> + scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
> }
Hmm, could you tell me what will happen if kallsyms_lookup_size_offset()
failed here?
Thanks,
> -#endif
> decoded = arm_probe_decode_insn(insn, asi);
>
> - if (decoded == INSN_REJECTED ||
> - is_probed_address_atomic(scan_start, scan_end))
> - return INSN_REJECTED;
> + if (decoded != INSN_REJECTED && scan_end)
> + if (is_probed_address_atomic(addr - 1, scan_end))
> + return INSN_REJECTED;
>
> return decoded;
> }
> --
> 2.5.0
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v4] arm64: Improve kprobes test for atomic sequence
From: David Long @ 2016-09-13 1:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913094629.6e8ca789f90ee7f2c80f33db@kernel.org>
On 09/12/2016 08:46 PM, Masami Hiramatsu wrote:
> On Mon, 12 Sep 2016 14:21:27 -0400
> David Long <dave.long@linaro.org> wrote:
>
>> From: "David A. Long" <dave.long@linaro.org>
>>
>> Kprobes searches backwards a finite number of instructions to determine if
>> there is an attempt to probe a load/store exclusive sequence. It stops when
>> it hits the maximum number of instructions or a load or store exclusive.
>> However this means it can run up past the beginning of the function and
>> start looking at literal constants. This has been shown to cause a false
>> positive and blocks insertion of the probe. To fix this, further limit the
>> backwards search to stop if it hits a symbol address from kallsyms. The
>> presumption is that this is the entry point to this code (particularly for
>> the common case of placing probes at the beginning of functions).
>>
>> This also improves efficiency by not searching code that is not part of the
>> function. There may be some possibility that the label might not denote the
>> entry path to the probed instruction but the likelihood seems low and this
>> is just another example of how the kprobes user really needs to be
>> careful about what they are doing.
>>
>> Signed-off-by: David A. Long <dave.long@linaro.org>
>> ---
>> arch/arm64/kernel/probes/decode-insn.c | 48 ++++++++++++++++------------------
>> 1 file changed, 23 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c
>> index 37e47a9..d1731bf 100644
>> --- a/arch/arm64/kernel/probes/decode-insn.c
>> +++ b/arch/arm64/kernel/probes/decode-insn.c
>> @@ -16,6 +16,7 @@
>> #include <linux/kernel.h>
>> #include <linux/kprobes.h>
>> #include <linux/module.h>
>> +#include <linux/kallsyms.h>
>> #include <asm/kprobes.h>
>> #include <asm/insn.h>
>> #include <asm/sections.h>
>> @@ -122,7 +123,7 @@ arm_probe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi)
>> static bool __kprobes
>> is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t *scan_end)
>> {
>> - while (scan_start > scan_end) {
>> + while (scan_start >= scan_end) {
>> /*
>> * atomic region starts from exclusive load and ends with
>> * exclusive store.
>> @@ -142,33 +143,30 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct arch_specific_insn *asi)
>> {
>> enum kprobe_insn decoded;
>> kprobe_opcode_t insn = le32_to_cpu(*addr);
>> - kprobe_opcode_t *scan_start = addr - 1;
>> - kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
>> -#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
>> - struct module *mod;
>> -#endif
>> -
>> - if (addr >= (kprobe_opcode_t *)_text &&
>> - scan_end < (kprobe_opcode_t *)_text)
>> - scan_end = (kprobe_opcode_t *)_text;
>> -#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
>> - else {
>> - preempt_disable();
>> - mod = __module_address((unsigned long)addr);
>> - if (mod && within_module_init((unsigned long)addr, mod) &&
>> - !within_module_init((unsigned long)scan_end, mod))
>> - scan_end = (kprobe_opcode_t *)mod->init_layout.base;
>> - else if (mod && within_module_core((unsigned long)addr, mod) &&
>> - !within_module_core((unsigned long)scan_end, mod))
>> - scan_end = (kprobe_opcode_t *)mod->core_layout.base;
>> - preempt_enable();
>> + kprobe_opcode_t *scan_end = NULL;
>> + unsigned long size = 0, offset = 0;
>> +
>> + /*
>> + * If there's a symbol defined in front of and near enough to
>> + * the probe address assume it is the entry point to this
>> + * code and use it to further limit how far back we search
>> + * when determining if we're in an atomic sequence. If we could
>> + * not find any symbol skip the atomic test altogether as we
>> + * could otherwise end up searching irrelevant text/literals.
>> + * KPROBES depends on KALLSYMS so this last case should never
>> + * happen.
>> + */
>> + if (kallsyms_lookup_size_offset((unsigned long) addr, &size, &offset)) {
>> + if (offset < (MAX_ATOMIC_CONTEXT_SIZE*sizeof(kprobe_opcode_t)))
>> + scan_end = addr - (offset / sizeof(kprobe_opcode_t));
>> + else
>> + scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
>> }
>
> Hmm, could you tell me what will happen if kallsyms_lookup_size_offset()
> failed here?
>
> Thanks,
>
>> -#endif
>> decoded = arm_probe_decode_insn(insn, asi);
>>
>> - if (decoded == INSN_REJECTED ||
>> - is_probed_address_atomic(scan_start, scan_end))
>> - return INSN_REJECTED;
>> + if (decoded != INSN_REJECTED && scan_end)
>> + if (is_probed_address_atomic(addr - 1, scan_end))
>> + return INSN_REJECTED;
>>
>> return decoded;
>> }
>> --
>> 2.5.0
>>
>
>
After the patch the function reads as follows:
> enum kprobe_insn __kprobes
> arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct arch_specific_insn *asi)
> {
> enum kprobe_insn decoded;
> kprobe_opcode_t insn = le32_to_cpu(*addr);
> kprobe_opcode_t *scan_end = NULL;
> unsigned long size = 0, offset = 0;
>
> /*
> * If there's a symbol defined in front of and near enough to
> * the probe address assume it is the entry point to this
> * code and use it to further limit how far back we search
> * when determining if we're in an atomic sequence. If we could
> * not find any symbol skip the atomic test altogether as we
> * could otherwise end up searching irrelevant text/literals.
> * KPROBES depends on KALLSYMS so this last case should never
> * happen.
> */
> if (kallsyms_lookup_size_offset((unsigned long) addr, &size, &offset)) {
> if (offset < (MAX_ATOMIC_CONTEXT_SIZE*sizeof(kprobe_opcode_t)))
> scan_end = addr - (offset / sizeof(kprobe_opcode_t));
> else
> scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
> }
> decoded = arm_probe_decode_insn(insn, asi);
>
> if (decoded != INSN_REJECTED && scan_end)
> if (is_probed_address_atomic(addr - 1, scan_end))
> return INSN_REJECTED;
>
> return decoded;
> }
A failed kallsyms_lookup_size_offset() call means scan_end will be left
as NULL, which in turn means arm_kprobe_decode_insn() will simply return
the result of the arm_probe_decode_insn() call. In other words it does
the normal analysis of the instruction to be probed, but does not do the
atomic sequence search that normally follows that (since it doesn't
really know how far back to search).
Thanks,
-dl
^ permalink raw reply
* [PATCH v5 0/3] Add ZTE ZX296718 support
From: Jun Nie @ 2016-09-13 1:42 UTC (permalink / raw)
To: linux-arm-kernel
Changes in version5:
- Add GPL license to new files.
- Unify clocks dts nodes names.
- Move interrupt parents info to root nodes in dts, move
pmu and timer node out of soc node.
Changes in version4:
- Change dts file name to align with compatible name.
- Reorder dts nodes with reg address sequence.
- Move nodes out of soc, which do not have soc simple
bus address or share soc interrupt.
- Change some nodes name to align with convention.
Changes in version3:
- Remove unnecessary flag of gic in dts.
Changes in version2:
- Use more precise cpu and pmu compatible name in dts.
- fix minor coding style issue.
Jun Nie (3):
arm64: add ZTE ZX SoC family
arm64: dts: Add ZTE ZX296718 SoC dts and Makefile
arm64: defconfig: enable ZTE ZX related config
Documentation/devicetree/bindings/arm/zte.txt | 24 +++
arch/arm64/Kconfig.platforms | 5 +
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/zte/Makefile | 5 +
arch/arm64/boot/dts/zte/zx296718-evb.dts | 25 +++
arch/arm64/boot/dts/zte/zx296718.dtsi | 254 ++++++++++++++++++++++++++
arch/arm64/configs/defconfig | 1 +
7 files changed, 315 insertions(+)
create mode 100644 arch/arm64/boot/dts/zte/Makefile
create mode 100644 arch/arm64/boot/dts/zte/zx296718-evb.dts
create mode 100644 arch/arm64/boot/dts/zte/zx296718.dtsi
--
1.9.1
^ permalink raw reply
* [PATCH v5 1/3] arm64: add ZTE ZX SoC family
From: Jun Nie @ 2016-09-13 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473730926-1034-1-git-send-email-jun.nie@linaro.org>
This patch introduces ARCH_ZX to add the support of the ZTE ZX SoC
family for the arm64 architecture.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
arch/arm64/Kconfig.platforms | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index be5d824..bc75fd2 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -204,6 +204,11 @@ config ARCH_XGENE
help
This enables support for AppliedMicro X-Gene SOC Family
+config ARCH_ZX
+ bool "ZTE ZX SoC Family"
+ help
+ This enables support for ZTE ZX SoC Family
+
config ARCH_ZYNQMP
bool "Xilinx ZynqMP Family"
help
--
1.9.1
^ permalink raw reply related
* [PATCH v5 2/3] arm64: dts: Add ZTE ZX296718 SoC dts and Makefile
From: Jun Nie @ 2016-09-13 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473730926-1034-1-git-send-email-jun.nie@linaro.org>
Add device tree support for ZX296718 SoC and evaluation board based
on it. Also document new values.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
Documentation/devicetree/bindings/arm/zte.txt | 24 +++
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/zte/Makefile | 5 +
arch/arm64/boot/dts/zte/zx296718-evb.dts | 29 +++
arch/arm64/boot/dts/zte/zx296718.dtsi | 260 ++++++++++++++++++++++++++
5 files changed, 319 insertions(+)
create mode 100644 arch/arm64/boot/dts/zte/Makefile
create mode 100644 arch/arm64/boot/dts/zte/zx296718-evb.dts
create mode 100644 arch/arm64/boot/dts/zte/zx296718.dtsi
diff --git a/Documentation/devicetree/bindings/arm/zte.txt b/Documentation/devicetree/bindings/arm/zte.txt
index 3ff5c9e..8336978 100644
--- a/Documentation/devicetree/bindings/arm/zte.txt
+++ b/Documentation/devicetree/bindings/arm/zte.txt
@@ -13,3 +13,27 @@ Low power management required properties:
Bus matrix required properties:
- compatible = "zte,zx-bus-matrix"
+
+
+---------------------------------------
+- ZX296718 SoC:
+ Required root node properties:
+ - compatible = "zte,zx296718"
+
+ZX296718 EVB board:
+ - "zte,zx296718-evb"
+
+System management required properties:
+ - compatible = "zte,zx296718-aon-sysctrl"
+ - compatible = "zte,zx296718-sysctrl"
+
+Example:
+aon_sysctrl: aon-sysctrl at 116000 {
+ compatible = "zte,zx296718-aon-sysctrl", "syscon";
+ reg = <0x116000 0x1000>;
+};
+
+sysctrl: sysctrl at 1463000 {
+ compatible = "zte,zx296718-sysctrl", "syscon";
+ reg = <0x1463000 0x1000>;
+};
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index 6e199c9..6684f97 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -19,6 +19,7 @@ dts-dirs += socionext
dts-dirs += sprd
dts-dirs += xilinx
dts-dirs += lg
+dts-dirs += zte
subdir-y := $(dts-dirs)
diff --git a/arch/arm64/boot/dts/zte/Makefile b/arch/arm64/boot/dts/zte/Makefile
new file mode 100644
index 0000000..6678066
--- /dev/null
+++ b/arch/arm64/boot/dts/zte/Makefile
@@ -0,0 +1,5 @@
+dtb-$(CONFIG_ARCH_ZX) += zx296718-evb.dtb
+
+always := $(dtb-y)
+subdir-y := $(dts-dirs)
+clean-files := *.dtb
diff --git a/arch/arm64/boot/dts/zte/zx296718-evb.dts b/arch/arm64/boot/dts/zte/zx296718-evb.dts
new file mode 100644
index 0000000..820fe1c
--- /dev/null
+++ b/arch/arm64/boot/dts/zte/zx296718-evb.dts
@@ -0,0 +1,29 @@
+/*
+ * ZTE Ltd. zx296718 Plaform
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ *
+ */
+/dts-v1/;
+#include "zx296718.dtsi"
+
+/ {
+ model = "ZTE zx296718 evaluation board";
+ compatible = "zte,zx296718-evb", "zte,zx296718";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory at 40000000 {
+ device_type = "memory";
+ reg = <0x40000000 0x40000000>;
+ };
+
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
new file mode 100644
index 0000000..f690e8f
--- /dev/null
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -0,0 +1,260 @@
+/*
+ * DTS File for ZTE ZX296718 Plaform
+ *
+ * Copyright (c) 2016 ZTE Semiconductor Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * publishhed by the Free Software Foundation.
+ *
+ */
+
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+ compatible = "zte,zx296718";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupt-parent = <&gic>;
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&cpu0>;
+ };
+ core1 {
+ cpu = <&cpu1>;
+ };
+ core2 {
+ cpu = <&cpu2>;
+ };
+ core3 {
+ cpu = <&cpu3>;
+ };
+ };
+ };
+
+ cpu0: cpu at 0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <0x0 0x0>;
+ enable-method = "psci";
+ };
+
+ cpu1: cpu at 1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <0x0 0x1>;
+ enable-method = "psci";
+ };
+
+ cpu2: cpu at 2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <0x0 0x2>;
+ enable-method = "psci";
+ };
+
+ cpu3: cpu at 3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53","arm,armv8";
+ reg = <0x0 0x3>;
+ enable-method = "psci";
+ };
+ };
+
+ clk24k: clk-24k {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24000>;
+ clock-output-names = "rtcclk";
+ };
+
+ osc32k: clk-osc32k {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <32000>;
+ clock-output-names = "osc32k";
+ };
+
+ osc12m: clk-osc12m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <12000000>;
+ clock-output-names = "osc12m";
+ };
+
+ osc24m: clk-osc24m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24000000>;
+ clock-output-names = "osc24m";
+ };
+
+ osc25m: clk-osc25m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <25000000>;
+ clock-output-names = "osc25m";
+ };
+
+ osc60m: clk-osc60m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <60000000>;
+ clock-output-names = "osc60m";
+ };
+
+ osc99m: clk-osc99m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <99000000>;
+ clock-output-names = "osc99m";
+ };
+
+ osc125m: clk-osc125m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <125000000>;
+ clock-output-names = "osc125m";
+ };
+
+ osc198m: clk-osc198m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <198000000>;
+ clock-output-names = "osc198m";
+ };
+
+ pll_audio: clk-pll-884m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <884000000>;
+ clock-output-names = "pll_audio";
+ };
+
+ pll_ddr: clk-pll-932m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <932000000>;
+ clock-output-names = "pll_ddr";
+ };
+
+ pll_hsic: clk-pll-960m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <960000000>;
+ clock-output-names = "pll_hsic";
+ };
+
+ pll_mac: clk-pll-1000m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1000000000>;
+ clock-output-names = "pll_mac";
+ };
+
+ pll_vga: clk-pll-1073m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1073000000>;
+ clock-output-names = "pll_vga";
+ };
+
+ pll_mm0: clk-pll-1188m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1188000000>;
+ clock-output-names = "pll_mm0";
+ };
+
+ pll_mm1: clk-pll-1296m {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <1296000000>;
+ clock-output-names = "pll_mm1";
+ };
+
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ pmu {
+ compatible = "arm,cortex-a53-pmu";
+ interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ gic: interrupt-controller at 2a00000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #address-cells = <0>;
+ #redistributor-regions = <6>;
+ redistributor-stride = <0x0 0x40000>;
+ interrupt-controller;
+ reg = <0x02a00000 0x10000>,
+ <0x02b00000 0x20000>,
+ <0x02b20000 0x20000>,
+ <0x02b40000 0x20000>,
+ <0x02b60000 0x20000>,
+ <0x02b80000 0x20000>,
+ <0x02ba0000 0x20000>;
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ soc {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "simple-bus";
+ ranges;
+
+ aon_sysctrl: aon-sysctrl at 116000 {
+ compatible = "zte,zx296718-aon-sysctrl", "syscon";
+ reg = <0x116000 0x1000>;
+ };
+
+ uart0: uart at 11f000 {
+ compatible = "arm,pl011", "arm,primecell";
+ arm,primecell-periphid = <0x001feffe>;
+ reg = <0x11f000 0x1000>;
+ interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc24m>;
+ clock-names = "apb_pclk";
+ status = "disabled";
+ };
+
+ dma: dma-controller at 1460000 {
+ compatible = "zte,zx296702-dma";
+ reg = <0x01460000 0x1000>;
+ interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc24m>;
+ clock-names = "dmaclk";
+ #dma-cells = <1>;
+ dma-channels = <32>;
+ dma-requests = <32>;
+ };
+
+ sysctrl: sysctrl at 1463000 {
+ compatible = "zte,zx296718-sysctrl", "syscon";
+ reg = <0x1463000 0x1000>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v5 3/3] arm64: defconfig: enable ZTE ZX related config
From: Jun Nie @ 2016-09-13 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473730926-1034-1-git-send-email-jun.nie@linaro.org>
This patch enables the configuration for the ZTE ZX family.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index eadf485..2bd5c88 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -57,6 +57,7 @@ CONFIG_ARCH_UNIPHIER=y
CONFIG_ARCH_VEXPRESS=y
CONFIG_ARCH_VULCAN=y
CONFIG_ARCH_XGENE=y
+CONFIG_ARCH_ZX=y
CONFIG_ARCH_ZYNQMP=y
CONFIG_PCI=y
CONFIG_PCI_MSI=y
--
1.9.1
^ permalink raw reply related
* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Dongdong Liu @ 2016-09-13 2:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473449047-10499-3-git-send-email-tn@semihalf.com>
Hi Tomasz
? 2016/9/10 3:24, Tomasz Nowicki ??:
> Some platforms may not be fully compliant with generic set of PCI config
> accessors. For these cases we implement the way to overwrite CFG accessors
> set and configuration space range.
>
> In first place pci_mcfg_parse() saves machine's IDs and revision number
> (these come from MCFG header) in order to match against known quirk entries.
> Then the algorithm traverses available quirk list (static array),
> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
> returns custom PCI config ops and/or CFG resource structure.
>
> When adding new quirk there are two possibilities:
> 1. Override default pci_generic_ecam_ops ops but CFG resource comes from MCFG
> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, MCFG_RES_EMPTY },
> 2. Override default pci_generic_ecam_ops ops and CFG resource. For this case
> it is also allowed get CFG resource from quirk entry w/o having it in MCFG.
> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops,
> DEFINE_RES_MEM(START, SIZE) },
>
> pci_generic_ecam_ops and MCFG entries will be used for platforms
> free from quirks.
>
> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
> drivers/acpi/pci_mcfg.c | 80 +++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index ffcc651..2b8acc7 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -32,6 +32,59 @@ struct mcfg_entry {
> u8 bus_start;
> u8 bus_end;
> };
> +struct mcfg_fixup {
> + char oem_id[ACPI_OEM_ID_SIZE + 1];
> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
> + u32 oem_revision;
> + u16 seg;
> + struct resource bus_range;
> + struct pci_ecam_ops *ops;
> + struct resource cfgres;
> +};
> +
> +#define MCFG_DOM_ANY (-1)
> +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \
> + ((end) - (start) + 1), \
> + NULL, IORESOURCE_BUS)
> +#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff)
> +#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0)
> +
> +static struct mcfg_fixup mcfg_quirks[] = {
> +/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
> +};
> +
> +static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> +static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> +static u32 mcfg_oem_revision;
> +
> +static void pci_mcfg_match_quirks(struct acpi_pci_root *root,
> + struct resource *cfgres,
> + struct pci_ecam_ops **ecam_ops)
> +{
> + struct mcfg_fixup *f;
> + int i;
> +
> + /*
> + * First match against PCI topology <domain:bus> then use OEM ID, OEM
> + * table ID, and OEM revision from MCFG table standard header.
> + */
> + for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> + if (f->seg == root->segment &&
why not use MCFG_DOM_RANGE, I think MCFG_DOM_RANGE is better.
if drop MCFG_DOM_RANGE, mcfg_quirks[] will be more complex.
static struct mcfg_fixup mcfg_quirks[] = {
/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
#ifdef CONFIG_PCI_HOST_THUNDER_ECAM
/* SoC pass1.x */
{ "CAVIUM", "THUNDERX", 2, 0, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 1, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 2, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 3, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 10, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 11, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 12, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
{ "CAVIUM", "THUNDERX", 2, 13, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
MCFG_RES_EMPTY},
#endif
.....
};
As PATCH v5 we only need define mcfg_quirks as below, It looks better.
static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook }, */
#ifdef CONFIG_PCI_HOST_THUNDER_PEM
/* Pass2.0 */
{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
thunder_pem_cfg_init },
{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
thunder_pem_cfg_init },
#endif
#ifdef CONFIG_PCI_HISI_ACPI
{ "HISI ", "HIP05 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
NULL, hisi_pcie_acpi_hip05_init},
{ "HISI ", "HIP06 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
NULL, hisi_pcie_acpi_hip06_init},
{ "HISI ", "HIP07 ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY,
NULL, hisi_pcie_acpi_hip07_init},
#endif
};
> + resource_contains(&f->bus_range, &root->secondary) &&
> + !memcmp(f->oem_id, mcfg_oem_id, ACPI_OEM_ID_SIZE) &&
> + !memcmp(f->oem_table_id, mcfg_oem_table_id,
> + ACPI_OEM_TABLE_ID_SIZE) &&
> + f->oem_revision == mcfg_oem_revision) {
> + if (f->cfgres.start)
> + *cfgres = f->cfgres;
> + if (f->ops)
> + *ecam_ops = f->ops;
> + dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
> + f->oem_id, f->oem_table_id, f->oem_revision);
> + return;
> + }
> + }
> +}
>
> /* List to save MCFG entries */
> static LIST_HEAD(pci_mcfg_list);
> @@ -61,14 +114,24 @@ int pci_mcfg_lookup(struct acpi_pci_root *root, struct resource *cfgres,
>
> }
>
> - if (!root->mcfg_addr)
> - return -ENXIO;
> -
> skip_lookup:
> memset(&res, 0, sizeof(res));
> - res.start = root->mcfg_addr + (bus_res->start << 20);
> - res.end = res.start + (resource_size(bus_res) << 20) - 1;
> - res.flags = IORESOURCE_MEM;
> + if (root->mcfg_addr) {
> + res.start = root->mcfg_addr + (bus_res->start << 20);
> + res.end = res.start + (resource_size(bus_res) << 20) - 1;
> + res.flags = IORESOURCE_MEM;
> + }
> +
> + /*
> + * Let to override default ECAM ops and CFG resource range.
> + * Also, this might even retrieve CFG resource range in case MCFG
> + * does not have it. Invalid CFG start address means MCFG firmware bug
> + * or we need another quirk in array.
> + */
> + pci_mcfg_match_quirks(root, &res, &ops);
> + if (!res.start)
> + return -ENXIO;
> +
> *cfgres = res;
> *ecam_ops = ops;
> return 0;
> @@ -101,6 +164,11 @@ static __init int pci_mcfg_parse(struct acpi_table_header *header)
> list_add(&e->list, &pci_mcfg_list);
> }
>
> + /* Save MCFG IDs and revision for quirks matching */
> + memcpy(mcfg_oem_id, header->oem_id, ACPI_OEM_ID_SIZE);
> + memcpy(mcfg_oem_table_id, header->oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
> + mcfg_oem_revision = header->revision;
> +
> pr_info("MCFG table detected, %d entries\n", n);
> return 0;
> }
>
^ permalink raw reply
* [PATCH v4 1/2] nxp/dts: add pcie aer interrupt-name property in the dts
From: Po Liu @ 2016-09-13 3:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912221358.GI23532@localhost>
Hi Bjorn, Shawn, Rob,
Thanks for reply. I'll upload new patches:
- separate the patches with arm and arm64 dts for new version.
- Modify the binding comments to make it clear for explain the 'aer' property.
Best regards,
Liu Po
-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org]
Sent: Tuesday, September 13, 2016 6:14 AM
To: Po Liu
Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; devicetree at vger.kernel.org; Bjorn Helgaas; Shawn Guo; Marc Zyngier; Rob Herring; Roy Zang; Vincent Hu; Stuart Yoder; Leo Li; Arnd Bergmann; M.H. Lian; Murali Karicheri
Subject: Re: [PATCH v4 1/2] nxp/dts: add pcie aer interrupt-name property in the dts
On Wed, Aug 31, 2016 at 02:37:21PM +0800, Po Liu wrote:
> NXP some platforms aer interrupt was not MSI/MSI-X/INTx but using
> interrupt line independently. This patch add a "aer"
> interrupt-names for aer interrupt.
> With the interrupt-names "aer", code could probe aer interrupt line
> for pcie root port, replace the aer interrupt service irq.
> This is intend to fixup the Layerscape platforms which aer interrupt
> was not MSI/MSI-X/INTx, but using interrupt line independently.
>
> Signed-off-by: Po Liu <po.liu@nxp.com>
Rob and Shawn had comments here, and I'm not sure they ever got resolved.
> ---
> changes for V4:
> - Add comments explain why to add this patch
> - Move the binding changes to pci code patch
>
> arch/arm/boot/dts/ls1021a.dtsi | 6 ++++--
> arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 18
> +++++++++--------- arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |
> 16 ++++++++--------
> 3 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/boot/dts/ls1021a.dtsi
> b/arch/arm/boot/dts/ls1021a.dtsi index 368e219..443e50b 100644
> --- a/arch/arm/boot/dts/ls1021a.dtsi
> +++ b/arch/arm/boot/dts/ls1021a.dtsi
> @@ -634,7 +634,8 @@
> reg = <0x00 0x03400000 0x0 0x00010000 /* controller registers */
> 0x40 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* controller interrupt */
> + interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> + interrupt-names = "aer";
> fsl,pcie-scfg = <&scfg 0>;
> #address-cells = <3>;
> #size-cells = <2>;
> @@ -657,7 +658,8 @@
> reg = <0x00 0x03500000 0x0 0x00010000 /* controller registers */
> 0x48 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> + interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
> + interrupt-names = "aer";
> fsl,pcie-scfg = <&scfg 1>;
> #address-cells = <3>;
> #size-cells = <2>;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index e669fbd..654071d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -527,9 +527,9 @@
> reg = <0x00 0x03400000 0x0 0x00100000 /* controller registers */
> 0x40 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 118 0x4>, /* controller interrupt */
> - <0 117 0x4>; /* PME interrupt */
> - interrupt-names = "intr", "pme";
> + interrupts = <0 117 0x4>, /* PME interrupt */
> + <0 118 0x4>; /* aer interrupt */
> + interrupt-names = "pme", "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> @@ -552,9 +552,9 @@
> reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
> 0x48 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 128 0x4>,
> - <0 127 0x4>;
> - interrupt-names = "intr", "pme";
> + interrupts = <0 127 0x4>,
> + <0 128 0x4>;
> + interrupt-names = "pme", "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> @@ -577,9 +577,9 @@
> reg = <0x00 0x03600000 0x0 0x00100000 /* controller registers */
> 0x50 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 162 0x4>,
> - <0 161 0x4>;
> - interrupt-names = "intr", "pme";
> + interrupts = <0 161 0x4>,
> + <0 162 0x4>;
> + interrupt-names = "pme", "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> index 21023a3..58844e8 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> @@ -583,8 +583,8 @@
> reg = <0x00 0x03400000 0x0 0x00100000 /* controller registers */
> 0x10 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 108 0x4>; /* Level high type */
> - interrupt-names = "intr";
> + interrupts = <0 108 0x4>; /* aer interrupt */
> + interrupt-names = "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> @@ -607,8 +607,8 @@
> reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
> 0x12 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 113 0x4>; /* Level high type */
> - interrupt-names = "intr";
> + interrupts = <0 113 0x4>; /* aer interrupt */
> + interrupt-names = "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> @@ -631,8 +631,8 @@
> reg = <0x00 0x03600000 0x0 0x00100000 /* controller registers */
> 0x14 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 118 0x4>; /* Level high type */
> - interrupt-names = "intr";
> + interrupts = <0 118 0x4>; /* aer interrupt */
> + interrupt-names = "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> @@ -655,8 +655,8 @@
> reg = <0x00 0x03700000 0x0 0x00100000 /* controller registers */
> 0x16 0x00000000 0x0 0x00002000>; /* configuration space */
> reg-names = "regs", "config";
> - interrupts = <0 123 0x4>; /* Level high type */
> - interrupt-names = "intr";
> + interrupts = <0 123 0x4>; /* aer interrupt */
> + interrupt-names = "aer";
> #address-cells = <3>;
> #size-cells = <2>;
> device_type = "pci";
> --
> 2.1.0.27.g96db324
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci"
> in the body of a message to majordomo at vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH V2 10/10] dmaengine: qcom_hidma: add MSI support for interrupts
From: kbuild test robot @ 2016-09-13 3:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473685384-19913-11-git-send-email-okaya@codeaurora.org>
Hi Sinan,
[auto build test ERROR on next-20160912]
[cannot apply to linus/master linux/master robh/for-next v4.8-rc6 v4.8-rc5 v4.8-rc4 v4.8-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/dmaengine-qcom_hidma-add-MSI-interrupt-support/20160912-215548
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All error/warnings (new ones prefixed by >>):
drivers/dma/qcom/hidma.c: In function 'hidma_msi_capable':
>> drivers/dma/qcom/hidma.c:690:3: error: implicit declaration of function 'acpi_device_hid' [-Werror=implicit-function-declaration]
ret = strcmp(acpi_device_hid(adev), "QCOM8062");
^
>> drivers/dma/qcom/hidma.c:690:9: warning: passing argument 1 of 'strcmp' makes pointer from integer without a cast
ret = strcmp(acpi_device_hid(adev), "QCOM8062");
^
In file included from include/linux/string.h:18:0,
from include/linux/bitmap.h:8,
from include/linux/cpumask.h:11,
from include/linux/rcupdate.h:40,
from include/linux/idr.h:18,
from include/linux/kernfs.h:14,
from include/linux/sysfs.h:15,
from include/linux/kobject.h:21,
from include/linux/device.h:17,
from include/linux/dmaengine.h:20,
from drivers/dma/qcom/hidma.c:45:
arch/xtensa/include/asm/string.h:63:19: note: expected 'const char *' but argument is of type 'int'
static inline int strcmp(const char *__cs, const char *__ct)
^
drivers/dma/qcom/hidma.c: At top level:
drivers/dma/qcom/hidma.c:558:20: warning: 'hidma_chirq_handler_msi' defined but not used [-Wunused-function]
static irqreturn_t hidma_chirq_handler_msi(int chirq, void *arg)
^
cc1: some warnings being treated as errors
vim +/acpi_device_hid +690 drivers/dma/qcom/hidma.c
684 &of_compat);
685 if (ret)
686 return false;
687
688 ret = strcmp(of_compat, "qcom,hidma-1.1");
689 } else {
> 690 ret = strcmp(acpi_device_hid(adev), "QCOM8062");
691 }
692
693 return ret == 0;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 46575 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160913/0837e931/attachment-0001.obj>
^ permalink raw reply
* [PATCH] ARM: dts: msm8974: Add definitions for QCE & cryptobam
From: Bjorn Andersson @ 2016-09-13 3:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160830153740.13275-1-voker57@gmail.com>
On Tue 30 Aug 08:37 PDT 2016, Iaroslav Gridin wrote:
> From: Voker57 <voker57@gmail.com>
>
> Add device tree definitions for Qualcomm Cryptography engine and its BAM
> Signed-off-by: Iaroslav Gridin <voker57@gmail.com>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 42 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 561d4d1..c0da739 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
> @@ -287,6 +287,48 @@
> reg = <0xf9011000 0x1000>;
> };
>
> + cryptobam: dma at fd444000 {
> + compatible = "qcom,bam-v1.4.0";
> + reg = <0xfd444000 0x15000>;
> + interrupts = <0 236 0>;
> + clocks = <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc GCC_CE2_CLK>;
> + clock-names = "bam_clk", "axi_clk", "core_clk";
> + #dma-cells = <1>;
> + qcom,ee = <1>;
> + qcom,controlled-remotely;
> + };
As Stan noted, please shift this '}' one step left (the rest looks well
indented.
> +
> + qcom,qcrypto at fd440000 {
Rename this "qcrypto" and make sure the address matches the reg
property.
> + compatible = "qcom,crypto-v5.1";
> + reg = <0xfd45a000 0x6000>;
> + reg-names = "crypto-base";
> + interrupts = <0 236 0>;
> + qcom,bam-pipe-pair = <2>;
> + qcom,ce-hw-instance = <1>;
> + qcom,ce-device = <0>;
> + clocks = <&gcc GCC_CE2_CLK>,
> + <&gcc GCC_CE2_AHB_CLK>,
> + <&gcc GCC_CE2_AXI_CLK>,
> + <&gcc CE2_CLK_SRC>;
> +
> + dmas = <&cryptobam 2>, <&cryptobam 3>;
> + dma-names = "rx", "tx";
> + clock-names = "core", "iface", "bus", "core_src";
> + qcom,clk-mgmt-sus-res;
> + qcom,msm-bus,name = "qcrypto-noc";
> +
> + qcom,msm-bus,num-cases = <2>;
> + qcom,msm-bus,num-paths = <1>;
> + qcom,use-sw-aes-cbc-ecb-ctr-algo;
> + qcom,use-sw-aes-xts-algo;
> + qcom,use-sw-ahash-algo;
> + qcom,msm-bus,vectors-KBps = <56 512 0 0>,
> + <56 512 3936000 393600>;
> + };
> +
> +
> timer at f9020000 {
It's nice to keep the nodes within a group ordered by address.
Regards,
Bjorn
^ permalink raw reply
* [PATCH v5 1/3] arm/dts: add pcie aer interrupt-name property in the dts
From: Po Liu @ 2016-09-13 4:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472625442-23309-2-git-send-email-po.liu@nxp.com>
NXP arm aer interrupt was not MSI/MSI-X/INTx but using interrupt
line independently. This patch add a "aer" interrupt-names for
aer interrupt.
With the interrupt-names "aer", code could probe aer interrupt line
for pcie root port, replace the aer interrupt service irq.
This patch is intend to fixup the Layerscape platforms which aer
interrupt was not MSI/MSI-X/INTx, but using interrupt line independently.
Signed-off-by: Po Liu <po.liu@nxp.com>
---
changes for v5:
- Seperate arm arm64 dts changes
arch/arm/boot/dts/ls1021a.dtsi | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 368e219..443e50b 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -634,7 +634,8 @@
reg = <0x00 0x03400000 0x0 0x00010000 /* controller registers */
0x40 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* controller interrupt */
+ interrupts = <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
+ interrupt-names = "aer";
fsl,pcie-scfg = <&scfg 0>;
#address-cells = <3>;
#size-cells = <2>;
@@ -657,7 +658,8 @@
reg = <0x00 0x03500000 0x0 0x00010000 /* controller registers */
0x48 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>; /* aer interrupt */
+ interrupt-names = "aer";
fsl,pcie-scfg = <&scfg 1>;
#address-cells = <3>;
#size-cells = <2>;
--
2.1.0.27.g96db324
^ permalink raw reply related
* [PATCH v5 2/3] arm64/dts: add pcie aer interrupt-name property in the dts
From: Po Liu @ 2016-09-13 4:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473741659-17618-1-git-send-email-po.liu@nxp.com>
Some platforms(NXP Layerscape for example) aer interrupt was not
MSI/MSI-X/INTx but using interrupt line independently. This patch
add a "aer" interrupt-names for aer interrupt.
With the interrupt-names "aer", code could probe aer interrupt line
for pcie root port, replace the aer interrupt service irq.
This is intend to fixup the Layerscape platforms which aer interrupt
was not MSI/MSI-X/INTx, but using interrupt line independently.
Signed-off-by: Po Liu <po.liu@nxp.com>
---
changes for v5:
- Seperate arm and arm64 dts modification into two patches
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 18 +++++++++---------
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 16 ++++++++--------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index e669fbd..654071d 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -527,9 +527,9 @@
reg = <0x00 0x03400000 0x0 0x00100000 /* controller registers */
0x40 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 118 0x4>, /* controller interrupt */
- <0 117 0x4>; /* PME interrupt */
- interrupt-names = "intr", "pme";
+ interrupts = <0 117 0x4>, /* PME interrupt */
+ <0 118 0x4>; /* aer interrupt */
+ interrupt-names = "pme", "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -552,9 +552,9 @@
reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
0x48 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 128 0x4>,
- <0 127 0x4>;
- interrupt-names = "intr", "pme";
+ interrupts = <0 127 0x4>,
+ <0 128 0x4>;
+ interrupt-names = "pme", "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -577,9 +577,9 @@
reg = <0x00 0x03600000 0x0 0x00100000 /* controller registers */
0x50 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 162 0x4>,
- <0 161 0x4>;
- interrupt-names = "intr", "pme";
+ interrupts = <0 161 0x4>,
+ <0 162 0x4>;
+ interrupt-names = "pme", "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 21023a3..58844e8 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -583,8 +583,8 @@
reg = <0x00 0x03400000 0x0 0x00100000 /* controller registers */
0x10 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 108 0x4>; /* Level high type */
- interrupt-names = "intr";
+ interrupts = <0 108 0x4>; /* aer interrupt */
+ interrupt-names = "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -607,8 +607,8 @@
reg = <0x00 0x03500000 0x0 0x00100000 /* controller registers */
0x12 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 113 0x4>; /* Level high type */
- interrupt-names = "intr";
+ interrupts = <0 113 0x4>; /* aer interrupt */
+ interrupt-names = "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -631,8 +631,8 @@
reg = <0x00 0x03600000 0x0 0x00100000 /* controller registers */
0x14 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 118 0x4>; /* Level high type */
- interrupt-names = "intr";
+ interrupts = <0 118 0x4>; /* aer interrupt */
+ interrupt-names = "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
@@ -655,8 +655,8 @@
reg = <0x00 0x03700000 0x0 0x00100000 /* controller registers */
0x16 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <0 123 0x4>; /* Level high type */
- interrupt-names = "intr";
+ interrupts = <0 123 0x4>; /* aer interrupt */
+ interrupt-names = "aer";
#address-cells = <3>;
#size-cells = <2>;
device_type = "pci";
--
2.1.0.27.g96db324
^ permalink raw reply related
* [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode
From: Po Liu @ 2016-09-13 4:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473741659-17618-1-git-send-email-po.liu@nxp.com>
On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
maybe there is interrupt line for aer pme etc. Search the interrupt
number in the fdt file. Then fixup the dev->irq with it.
Signed-off-by: Po Liu <po.liu@nxp.com>
---
changes for v5:
- Add clear 'aer' interrup-names description
.../devicetree/bindings/pci/layerscape-pci.txt | 11 +++++---
drivers/pci/pcie/portdrv_core.c | 31 +++++++++++++++++++---
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
index 41e9f55..101d0a7 100644
--- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
+++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
@@ -18,8 +18,10 @@ Required properties:
- reg: base addresses and lengths of the PCIe controller
- interrupts: A list of interrupt outputs of the controller. Must contain an
entry for each entry in the interrupt-names property.
-- interrupt-names: Must include the following entries:
- "intr": The interrupt that is asserted for controller interrupts
+- interrupt-names: It may be include the following entries:
+ "aer": The interrupt that is asserted for aer interrupt
+ "pme": The interrupt that is asserted for pme interrupt
+ ......
- fsl,pcie-scfg: Must include two entries.
The first entry must be a link to the SCFG device node
The second entry must be '0' or '1' based on physical PCIe controller index.
@@ -35,8 +37,9 @@ Example:
reg = <0x00 0x03400000 0x0 0x00010000 /* controller registers */
0x40 0x00000000 0x0 0x00002000>; /* configuration space */
reg-names = "regs", "config";
- interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* controller interrupt */
- interrupt-names = "intr";
+ interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, /* aer interrupt */
+ <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* pme interrupt */
+ interrupt-names = "aer", "pme";
fsl,pcie-scfg = <&scfg 0>;
#address-cells = <3>;
#size-cells = <2>;
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index e9270b4..7c4943d 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -16,6 +16,7 @@
#include <linux/slab.h>
#include <linux/pcieport_if.h>
#include <linux/aer.h>
+#include <linux/of_irq.h>
#include "../pci.h"
#include "portdrv.h"
@@ -200,6 +201,28 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
{
int i, irq = -1;
+ int ret;
+ struct device_node *np = NULL;
+
+ for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
+ irqs[i] = 0;
+
+ if (dev->bus->dev.of_node)
+ np = dev->bus->dev.of_node;
+
+ /* If root port doesn't support MSI/MSI-X/INTx in RC mode,
+ * request irq for aer
+ */
+ if (IS_ENABLED(CONFIG_OF_IRQ) && np &&
+ (mask & PCIE_PORT_SERVICE_PME)) {
+ ret = of_irq_get_byname(np, "aer");
+ if (ret > 0) {
+ irqs[PCIE_PORT_SERVICE_AER_SHIFT] = ret;
+ if (dev->irq)
+ irq = dev->irq;
+ goto no_msi;
+ }
+ }
/*
* If MSI cannot be used for PCIe PME or hotplug, we have to use
@@ -225,11 +248,13 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
irq = dev->irq;
no_msi:
- for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
- irqs[i] = irq;
+ for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
+ if (!irqs[i])
+ irqs[i] = irq;
+ }
irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
- if (irq < 0)
+ if (irq < 0 && irqs[PCIE_PORT_SERVICE_AER_SHIFT] < 0)
return -ENODEV;
return 0;
}
--
2.1.0.27.g96db324
^ permalink raw reply related
* [PATCH 2/3] ARM64: dts: amlogic: Add basic support for Amlogic S905X
From: Kevin Hilman @ 2016-09-13 4:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <28d9160a-ce40-cbbb-b4e9-3ec34be52368@suse.de>
On Mon, Sep 12, 2016 at 2:43 PM, Andreas F?rber <afaerber@suse.de> wrote:
> Am 12.09.2016 um 22:43 schrieb Kevin Hilman:
>> Carlo Caione <carlo@caione.org> writes:
>>> On Mon, Sep 12, 2016 at 6:28 PM, Andreas F?rber <afaerber@suse.de> wrote:
>>>
>>>>> +Boards with the Amlogic Meson GXL SoC shall have the following properties:
>>>>> + Required root node property:
>>>>> + compatible: "amlogic,meson-gxl-s905x", "amlogic,meson-gxl";
>>>>
>>>> Can we please use "amlogic,s905x", "amlogic,meson-gxl"? No need to
>>>> complicate the name. Also affects .dtsi and .dts below.
>>>
>>> gxl != s905x.
>
> Huh? You're seemingly completely missing my point...
>
> But you are right that _Neil's_ heading needs to be fixed, too:
> Clearly not all GXL SoCs need to have an S905X compatible string!
> So it should be "Boards with the Amlogic S905X SoC shall ..." or so.
>
>>> AFAWK to the GXL family belong several different SoCs, like S905X,
>>> S905D, etc... (see patch 3/3)
>
> Thanks, I already know that, that's why you have two compatible strings
> instead of just one like for GXBB. We can certainly prepend one for
> symmetry there, too, if it makes you happier.
>
>>> This is why we use meson-gxl-s905x, meson-gxl-s905d, etc...
>>
>> Correct.
>>
>>> We could s/meson-gxl-s905x/meson-s905x/ and
>>> s/meson-gxl-s905d/meson-s905d/ but I honestly prefer this way because
>>> we can clearly see which family the SoC belongs to (the Amlogic naming
>>> convention is already messy enough).
>>> I mean, yes it's longer, but it's for the sake of documentation IMO.
>>
>> +1
>
> I still don't follow that conclusion. The board is called "amlogic,p231"
> because P231 is a unique identifier within the Amlogic namespace, so why
> not call the SoC "amlogic,s905x" for the same reason? The documentation
> is already there in having both "amlogic,s905x" _and_
> "amlogic,meson-gxl" - please re-read my post. There is no S905X in GXL
> family and another S905X in some other Amlogic SoC family, so it's
> unique and there is no reason to encode any hierarchies into its name
> other than vendor,name.
>
> I'm not arguing over the file name, where it perfectly makes sense to
> have a meson-gxl- prefix (already discussed), just about the compatible
> string where we don't have "amlogic,meson-gxl-s905x-p231" either because
> it is completely unnecessary and does _not_ add any value.
Sorry, I'm guilty of not fully reading the original post. I was
thinking only of the filenames, which it seems were already agreed on
to be long.
> Not that we're checking this string anywhere anyway... If you want to
> check for the GXL family you have to use "amlogic,meson-gxl"; if you
> want to check for the specific SoC you use "amlogic,s905x". Simple. We
> never match partial strings, so there is no sense in a hardcoded prefix
> that is duplicating information already available.
For the compatible strings, I think your proposed shorter (but still
unique) forms are fine with me.
Kevin
^ permalink raw reply
* [PATCH v7 9/9] drm/mediatek: add support for Mediatek SoC MT2701
From: CK Hu @ 2016-09-13 5:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473675413.12398.29.camel@mtksdaap41>
Hi, YT:
On Mon, 2016-09-12 at 18:16 +0800, YT Shen wrote:
> Hi CK,
>
> On Wed, 2016-09-07 at 13:37 +0800, CK Hu wrote:
> > Hi, YT:
> >
> > On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote:
> > > This patch add support for the Mediatek MT2701 DISP subsystem.
> > > There is only one OVL engine in MT2701.
> > >
> > > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> >
> > [snip...]
> >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > index 4b4e449..465819b 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
> > > @@ -112,6 +112,7 @@ struct mtk_ddp_comp_match {
> > >
> > > static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_ID_MAX] = {
> > > [DDP_COMPONENT_AAL] = { MTK_DISP_AAL, 0, NULL },
> > > + [DDP_COMPONENT_BLS] = { MTK_DISP_PWM, 0, NULL },
> >
> > I think BLS is different than PWM, so this statement should be
> >
> > [DDP_COMPONENT_BLS] = { MTK_DISP_BLS, 0, NULL };
> The BLS module actually is a multifunction device, one of them is the
> PWM function. We only upstream PWM function [1] now, and it is
> accepted. When there are real use case (gamma function), we will update
> this part. What do you think?
I think BLS = PWM + GAMMA and the device with register range from
0x1400a000 to 0x1400afff should be called BLS. I think this device is
called PWM in [1] because it just use its PWM function and it's not
suitable. At least in DRM driver, we should use the term BLS rather than
PWM. Maybe we should define as below:
[DDP_COMPONENT_BLS] = { MTK_DISP_BLS, 0, NULL };
and
{ .compatible = "mediatek,mt2701-disp-pwm", .data = (void
*)MTK_DISP_BLS },
Regards,
CK
[1] https://patchwork.kernel.org/patch/9223001/
>
> Regards,
> yt.shen
>
> [1] https://patchwork.kernel.org/patch/9223001/
>
> >
> >
> > > [DDP_COMPONENT_COLOR0] = { MTK_DISP_COLOR, 0, &ddp_color },
> > > [DDP_COMPONENT_COLOR1] = { MTK_DISP_COLOR, 1, &ddp_color },
> > > [DDP_COMPONENT_DPI0] = { MTK_DPI, 0, NULL },
> >
> > Regards,
> > CK
> >
> >
>
>
^ permalink raw reply
* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Tomasz Nowicki @ 2016-09-13 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADaLNDkforSVOADi_GpYGF7GG3M1tcdmqsGSx=F1BD3FgETHoQ@mail.gmail.com>
On 13.09.2016 00:47, Duc Dang wrote:
> On Mon, Sep 12, 2016 at 3:24 PM, Duc Dang <dhdang@apm.com> wrote:
>> On Fri, Sep 9, 2016 at 12:24 PM, Tomasz Nowicki <tn@semihalf.com> wrote:
>>>
>>> Some platforms may not be fully compliant with generic set of PCI config
>>> accessors. For these cases we implement the way to overwrite CFG accessors
>>> set and configuration space range.
>>>
>>> In first place pci_mcfg_parse() saves machine's IDs and revision number
>>> (these come from MCFG header) in order to match against known quirk entries.
>>> Then the algorithm traverses available quirk list (static array),
>>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>>> returns custom PCI config ops and/or CFG resource structure.
>>>
>>> When adding new quirk there are two possibilities:
>>> 1. Override default pci_generic_ecam_ops ops but CFG resource comes from MCFG
>>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops, MCFG_RES_EMPTY },
>>> 2. Override default pci_generic_ecam_ops ops and CFG resource. For this case
>>> it is also allowed get CFG resource from quirk entry w/o having it in MCFG.
>>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops,
>>> DEFINE_RES_MEM(START, SIZE) },
>>>
>>> pci_generic_ecam_ops and MCFG entries will be used for platforms
>>> free from quirks.
>>>
>>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>>> ---
>>> drivers/acpi/pci_mcfg.c | 80 +++++++++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 74 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index ffcc651..2b8acc7 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -32,6 +32,59 @@ struct mcfg_entry {
>>> u8 bus_start;
>>> u8 bus_end;
>>> };
>>> +struct mcfg_fixup {
>>> + char oem_id[ACPI_OEM_ID_SIZE + 1];
>>> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>>> + u32 oem_revision;
>>> + u16 seg;
>>> + struct resource bus_range;
>>> + struct pci_ecam_ops *ops;
>>> + struct resource cfgres;
>>> +};
>>> +
>>> +#define MCFG_DOM_ANY (-1)
>>> +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \
>>> + ((end) - (start) + 1), \
>>> + NULL, IORESOURCE_BUS)
>>> +#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff)
>>> +#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0)
>>> +
>>> +static struct mcfg_fixup mcfg_quirks[] = {
>>> +/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
>>> +};
>>> +
>>> +static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>> +static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
>>> +static u32 mcfg_oem_revision;
>>> +
>>> +static void pci_mcfg_match_quirks(struct acpi_pci_root *root,
>>> + struct resource *cfgres,
>>> + struct pci_ecam_ops **ecam_ops)
>>> +{
>>> + struct mcfg_fixup *f;
>>> + int i;
>>> +
>>> + /*
>>> + * First match against PCI topology <domain:bus> then use OEM ID, OEM
>>> + * table ID, and OEM revision from MCFG table standard header.
>>> + */
>>> + for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
>>> + if (f->seg == root->segment &&
>>
>> Is dropping the comparison with MCFG_DOM_ANY intended? It is useful if
>> all the controllers (segs) can use the same quirk (X-Gene case).
>> If you decide to drop it, then we can remove MCFG_DOM_ANY definition as well.
>>
>>> + resource_contains(&f->bus_range, &root->secondary) &&
>>> + !memcmp(f->oem_id, mcfg_oem_id, ACPI_OEM_ID_SIZE) &&
>>> + !memcmp(f->oem_table_id, mcfg_oem_table_id,
>>> + ACPI_OEM_TABLE_ID_SIZE) &&
>>> + f->oem_revision == mcfg_oem_revision) {
>>> + if (f->cfgres.start)
>>> + *cfgres = f->cfgres;
>>> + if (f->ops)
>>> + *ecam_ops = f->ops;
>>> + dev_info(&root->device->dev, "Applying PCI MCFG quirks for %s %s rev: %d\n",
>>> + f->oem_id, f->oem_table_id, f->oem_revision);
>>> + return;
>>> + }
>>> + }
>>> +}
>>>
>>> /* List to save MCFG entries */
>>> static LIST_HEAD(pci_mcfg_list);
>>> @@ -61,14 +114,24 @@ int pci_mcfg_lookup(struct acpi_pci_root *root, struct resource *cfgres,
>>>
>>> }
>>>
>>> - if (!root->mcfg_addr)
>>> - return -ENXIO;
>>> -
>>> skip_lookup:
>>> memset(&res, 0, sizeof(res));
>>> - res.start = root->mcfg_addr + (bus_res->start << 20);
>>> - res.end = res.start + (resource_size(bus_res) << 20) - 1;
>>> - res.flags = IORESOURCE_MEM;
>>> + if (root->mcfg_addr) {
>>> + res.start = root->mcfg_addr + (bus_res->start << 20);
>>> + res.end = res.start + (resource_size(bus_res) << 20) - 1;
>>> + res.flags = IORESOURCE_MEM;
>>> + }
>>> +
>>> + /*
>>> + * Let to override default ECAM ops and CFG resource range.
>>> + * Also, this might even retrieve CFG resource range in case MCFG
>>> + * does not have it. Invalid CFG start address means MCFG firmware bug
>>> + * or we need another quirk in array.
>>> + */
>>> + pci_mcfg_match_quirks(root, &res, &ops);
>>> + if (!res.start)
>>> + return -ENXIO;
>>> +
>>> *cfgres = res;
>>> *ecam_ops = ops;
>>> return 0;
>>> @@ -101,6 +164,11 @@ static __init int pci_mcfg_parse(struct acpi_table_header *header)
>>> list_add(&e->list, &pci_mcfg_list);
>>> }
>>>
>>> + /* Save MCFG IDs and revision for quirks matching */
>>> + memcpy(mcfg_oem_id, header->oem_id, ACPI_OEM_ID_SIZE);
>>> + memcpy(mcfg_oem_table_id, header->oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
>>> + mcfg_oem_revision = header->revision;
>
> I think this is a typo, it should be:
> mcfg_oem_revision = header->oem_revision;
You are right, header->oem_revision should be assigned.
Thanks!
Tomasz
^ permalink raw reply
* [PATCH V2 1/4] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang @ 2016-09-13 6:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <246257868.K0dkmX9SMq@wuerfel>
Hi, Arnd,
On 2016?09?08? 21:23, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 3:45:21 PM CEST zhichang.yuan wrote:
>> On 2016/9/7 23:06, Arnd Bergmann wrote:
>>> On Wednesday, September 7, 2016 9:33:50 PM CEST Zhichang Yuan wrote:
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> +
>>>> +typedef u64 (*inhook)(void *devobj, unsigned long ptaddr, void *inbuf,
>>>> + size_t dlen, unsigned int count);
>>>> +typedef void (*outhook)(void *devobj, unsigned long ptaddr,
>>>> + const void *outbuf, size_t dlen,
>>>> + unsigned int count);
>>>> +
>>>> +struct extio_ops {
>>>> + inhook pfin;
>>>> + outhook pfout;
>>>> + void *devpara;
>>>> +};
>>>> +
>>>> +extern struct extio_ops *arm64_simops __refdata;
>>>> +
>>>> +/*Up to now, only applied to Hip06 LPC. Define as static here.*/
>>>> +static inline void arm64_set_simops(struct extio_ops *ops)
>>>> +{
>>>> + if (ops)
>>>> + WRITE_ONCE(arm64_simops, ops);
>>>> +}
>>>> +
>>>> +
>>>> +#define BUILDIO(bw, type) \
>>>> +static inline type in##bw(unsigned long addr) \
>>>> +{ \
>>>> + if (addr >= PCIBIOS_MIN_IO) \
>>>> + return read##bw(PCI_IOBASE + addr); \
>>>> + return (arm64_simops && arm64_simops->pfin) ? \
>>>> + arm64_simops->pfin(arm64_simops->devpara, addr, NULL, \
>>>> + sizeof(type), 1) : -1; \
>>>> +} \
>>>>
>>>
>>> Hmm, the way this is done, enabling CONFIG_ARM64_INDIRECT_PIO at
>>> compile time means that only the dynamically registered PIO support
>>> is possible for I/O port ranges 0-0xfff.
>> Yes. The arm64_simops is only for IO range 0-0xfff. But since only one global arm64_simops, this patch doesn't
>> support the dynamically PIO register, only one PIO range of 0-0xfff is supported. As for multiple PIO ranges
>> register, you also mention below, will discuss there.
>
> I think having only one range is enough, but it may be best not to
> assume that this is mapped at a fixed location in the Linux PIO
> port address space.
ok. I will add the linux PIO range into struct extio_ops. With this new added PIO range, we can register a
variable linux PIO range to the global arm64_simops for our LPC.
>
> As I understand, you list all the child devices in DT, so those
> port numbers should all be translatable from bus specific (0x0-0xfff)
> into the larger Linux range that also contains PCI devices.
>
>>> I think the runtime check should better test if simops was defined
>>> first and fall back to normal PIO otherwise, in order to allow
>>> LPC implementations on a PCI-LPC bridge.
>> Do you mean check arm64_simops first?
>> I don't understand clearly what is the benefit about that.
>> It seems that most IO accesses are MMIO, is it the current implementation a bit efficent?
>
> No, this is about having devices at hardcoded PIO addresses behind PCI
> on another (non-hisilicon) machine running the same kernel binary.
>
What about defining inb like that:
static inline u8 inb(unsigned long addr)
{
#ifdef CONFIG_ARM64_INDIRECT_PIO
if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
addr <= arm64_extio_ops->end)
return extio_inb(addr);
#endif
return readb(PCI_IOBASE + addr);
}
The CONFIG_ARM64_INDIRECT_PIO is still using. When indirect-IO is needed, ARM64_INDIRECT_PIO will be selected.
I don't want to define arm64_extio_ops in some build-in source file, such as setup.c; keeping
CONFIG_ARM64_INDIRECT_PIO is for the specific devices which need indirect-IO.
extio_inb is defined as weak function in c file.
All these revise will be presented in V3 patch soon.
>>> How about allowing an I/O port range to be defined along with
>>> the operations and check against that?
>>>
>>> u8 intb(unsigned long port)
>>> {
>>> if (arm64_simops &&
>>> (port >= arm64_simops->min) &&
>>> (port <= arm64_simops->max))
>>> return arm64_simops->pfin(arm64_simops, port, 1);
>>> else
>>> return readb(PCI_IOBASE + addr);
>>> }
>>>
>>> The other advantage of that is that you can dynamically register
>>> a translation for the LPC port range into the Linux I/O port range
>>> like PCI hosts do.
>> Yes. an IO port range along with the operations is more generic and extensible.
>> Do you want to define extio_ops like that:
>>
>> struct extio_ops {
>> unsigned long start;
>> unsigned long end;
>> unsigned long ptoffset;/* port IO - linux io */
>> inhook pfin;
>> outhook pfout;
>> void *devpara;
>> };
>>
>> With this structure, we can register the PIO range we need without limit in 0-0xfff. But there is only one global struct
>> extio_ops where arm64_simops points to, we can only register one operation.
>> Actually, Hip06 LPC currently need at least two PIO ranges, 0xe4-0xe7, 0x2f8-0x2ff.
>> In this patch, we want to make the PIO differentiation in the new revised in/out() is more simpler, just reserve a bigger
>> PIO range of 0-0xfff from the whole PIO range for this indirect-IO introduced in this patch-set. I think this reservation
>> is not so safe, if there are other legacy devices which are designed to use fixable PIO range below 0x1000 through in/out,
>> the trouble will happen.
>
> I don't think we can solve all the possible cases. When a driver asks
> for a hardcoded address, we can either route that to the first PCI bus
> that registers, or always route it to LPC, but there may always be
> corner cases that don't work.
>
> Fortunately, it is very rare for hardcoded PIO addresses to be required
> in particular on non-x86 architectures, so it might not matter too much
> in practice:
>
> Having the extio range live on ports 0-0x1000 by default is probably
> reasonable, as long as that range is also usable for PCI on other
> platforms. Having it registered dynamically after the PCI bus should
> also be ok.
>
In the coming patch V3, we don't reserve the fixed PIO range of 0-0x1000. The extio range is totally depended
on the device IO resource configuration.
Several changes will be done in V3:
1) adopt the translation for all IO ranges including those from Hip06 LPC by pci_address_to_pio. Which means
that all physical IO ranges will be converted to fully different linux PIO ranges. Based on this, I think the
PCI PIO ranges can co-exist with the PIO ranges from other bus, including Hip06 LPC;
2) Since only one extio range is supported in this patch-set, we only register the linux PIO range for IPMI bt
in arm64_simops to work based on indirect-IO; In this way, ipmi bt can work well without any changes on
current ipmi driver.
>> Based on your initial idea, I have two thoughts which help to make the indirect-IO more generic:
>>
>> 1. setup a list where all indirect-IO devices' operations are linked to
>>
>>
>> struct extio_range {
>> unsigned long start;/* inclusive, sys io addr */
>> unsigned long end;/* inclusive, sys io addr */
>> unsigned long ptoffset;/* port Io - system Io */
>> };
>>
>> struct extio_node {
>> struct list_head ranlink;
>>
>> struct extio_range iores;
>>
>> /*pointer to the device provided services*/
>> struct extio_ops *regops;
>> };
>>
>> when in/out is called with the input PIO parameter, check which node contains the input PIO and call the corresponding operation to
>> complete the IO.
>>
>> static inline type inb(unsigned long addr)
>> {
>> struct extio_node *extop;
>> unsigned long offset;
>> /* extio_range_getops() will scan the list to find the node where start <= addr <= end is satisfied*/
>> extop = extio_range_getops(addr, &offset);
>> if (!extop)
>> return read##bw(PCI_IOBASE + addr);
>> if (extop->regops && extop->regops->pfin)
>> return extop->regops->pfin(extop->regops->devpara,
>> addr + offset, NULL, sizeof(type), 1);
>> return -1;
>> }
>>
>> The major disadvantage of this method is the performance. When the list is not long, it will be ok, I think.
>
>> If support multiple PIO ranges are not needed, we don't need this list, only continue use the global arm64_simops based on the new
>> extio_ops structure. Probably this is your suggestion.
>
> I wouldn't go this far: just assume that there is either one set of
> operations registered or none at all, but make it possible to have it
> at an arbitrary address.
>
>> 2. extend the linux IO space to spare a fully separate PIO range for indirect-IO
>>
>> the current linux IO space on arm64 is 0 to IO_SPACE_LIMIT:
>>
>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>
>> current PCI_IO_SIZE is 16M.
>>
>> It seems the current linux IO space on arm64 is totally for PCI IO based on MMIO. For indirect-IO in this patch-set, we populate the linux
>> IO range from 16M to 18M, this 2M linux IO space can be divided into 32 segments with segment size is 64K. Each segment is exclusively populated
>> by one indirect-IO device. when the device is creating, a segment with unique segment ID will be allocated and the IO resource will be converted
>> to the IO range corresponding with that segment. For example, segement 2 will own the IO range 0x1020000 - 0x102ffff.
>>
>> the structure for this way is:
>>
>> #define EXTIO_VECTOR_MAX 32
>> struct extio_vector {
>> struct mutex seglock;
>>
>> /* one bit corresponds with one segment */
>> DECLARE_BITMAP(bmap, EXTIO_VECTOR_MAX);
>> struct extio_ops *opsvec;
>> };
>>
>>
>> when the corresponding driver call in/out with one port address from the allocated linux IO resource, the processing like that:
>>
>> static inline type inb(unsigned long addr)
>> {
>> if (!(addr & (0x01 << 16))) /* only check bit 16 */
>> return readb(PCI_IOBASE + addr);
>> /* extio_inb will directly parse the bit16 to bit 20 to get the segment ID, then get the corresponding IO operation specific to device */
>> return extio_inb(addr);
>> }
>>
>> This method is nearly no performance lose, but is more complicated. Maybe it is not worthy to do that.
>
> No, I don't think this is necessary either.
Ok. Will make the code simpler. One extio range is enough for hip06 LPC at this moment.
Thanks,
Zhichang
>
>>> We may also want to move the inb/outb definitions into a .c file
>>> as they are getting rather big.
>> The current in/out is defined as inline function in asm-generic/io.h; If we move them to .c file, probably much change.....
>
> The current implementation turns into a single CPU instruction, my idea
> was not to grow that too much but instead turn it into a branch instruction
> when your code is enabled at compile-time. When it's disabled, we should
> still use the existing behavior.
>
> Arnd
>
^ permalink raw reply
* [PATCH 1/8] ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.
From: Eric Anholt @ 2016-09-13 6:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7f432ebf-78d9-9089-f79d-d0584de89887@destevenson.freeserve.co.uk>
Dave Stevenson <linux-rpi-kernel@destevenson.freeserve.co.uk> writes:
> On 09/09/16 22:20, Eric Anholt wrote:
>> e<#secure method=pgpmime mode=sign>
>> Gerd Hoffmann <kraxel@redhat.com> writes:
>>
>>> Hi,
>>>
>>>> According to this page [1] the pinctrl group for parallel display interface is
>>>> missing. Is it intended?
>>>>
>>>> [1] - http://elinux.org/RPi_BCM2835_GPIOs
>>> Just an oversight I guess. Eric?
>>>
>>> Does this look correct?
>>>
>>> + dpi_gpio4: dpi_gpio4 {
>>> + brcm,pins = <4 5 6 7 8 9 10 11 12 13
>>> + 14 15 16 17 18 19 20 21
>>> + 22 23 24 25 26 27>;
>>> + brcm,function = <BCM2835_FSEL_ALT2>;
>>> + };
>> For DPI, you also need pins 0-3 in there for clock and syncs.
>>
>> That set of data pins would be for a 24-bit mode, which is what we
>> should be using for the Adafruit kippah + 7" panel combo.
> The Kippah is only 18bit, RGB666.
> https://www.adafruit.com/products/2454
> "The pins used are GPIO 2 through 21 inclusive. That means you don't get
> the UART RX/TX pins (no console cable) and you don't get the standard
> user I2C pins, the EEPROM I2C pins, or hardware SPI pins. You do get to
> use pins #22, #23, #24, #25, #26 and #27, and the USB ports are fine to
> use too."
I was confused because my pinctrl was set up for 18, but the panel
driver for it is set up for 24. It looks like 18 is right, so I should
probably correct the panel driver.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160912/a143f3d2/attachment.sig>
^ permalink raw reply
* [PATCH 2/3] ARM64: dts: amlogic: Add basic support for Amlogic S905X
From: Carlo Caione @ 2016-09-13 6:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <28d9160a-ce40-cbbb-b4e9-3ec34be52368@suse.de>
On Mon, Sep 12, 2016 at 11:43 PM, Andreas F?rber <afaerber@suse.de> wrote:
[cut]
> I'm not arguing over the file name, where it perfectly makes sense to
> have a meson-gxl- prefix (already discussed), just about the compatible
> string where we don't have "amlogic,meson-gxl-s905x-p231" either because
> it is completely unnecessary and does _not_ add any value.
>
> Not that we're checking this string anywhere anyway... If you want to
> check for the GXL family you have to use "amlogic,meson-gxl"; if you
> want to check for the specific SoC you use "amlogic,s905x". Simple. We
> never match partial strings, so there is no sense in a hardcoded prefix
> that is duplicating information already available.
Ok, then. Fine with me.
Neil, do you want to resend my patch or you can take care of the fixes
for the whole patchset?
Thanks,
--
Carlo Caione
^ permalink raw reply
* [PATCH v2 2/6] ARM: dts: bcm283x: Define standard pinctrl groups in the gpio node.
From: Gerd Hoffmann @ 2016-09-13 6:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1273800480.83718.15854341-2f8e-4e20-95f4-e76f81e6fd56.open-xchange@email.1und1.de>
On Mo, 2016-09-12 at 20:19 +0200, Stefan Wahren wrote:
> Hi Gerd,
>
> > Gerd Hoffmann <kraxel@redhat.com> hat am 12. September 2016 um 10:22
> > geschrieben:
> >
> >
> > From: Eric Anholt <eric@anholt.net>
> >
> > The BCM2835-ARM-Peripherals.pdf documentation specifies what the
> > function selects do for the pins, and there are a bunch of obvious
> > groupings to be made. With these created, we'll be able to replace
> > bcm2835-rpi.dtsi's main "set all of these pins to alt0" with
> > references to specific groups we want enabled.
> >
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> >
> > squashed in:
> > ARM: dts: bcm283x: Add the emmc pin group to bcm283x.dtsi.
> > ARM: dts: bcm283x: Add a group for mapping pins 48-53 to sdhost.
> > ARM: dts: bcm283x: Add a new EMMC pin group from the downstream tree.
> >
> > fixups by kraxel:
> > * fix spi0 name
> > * sort & group entries
> > * use pull defines
> > * add dpi group
>
> this looks like a changelog.
It actually is, trying to keep track of what comes from Eric and what
comes from me. But I guess I can also simply summarize that as "based
on a patch from Eric".
> > + dpi_gpio4: dpi_gpio4 {
> > + brcm,pins = <0 1 2 3 4 5 6 7 8 9 10 11
> > + 12 13 14 15 16 17 18 19
> > + 20 21 22 23 24 25 26 27>;
> > + brcm,function = <BCM2835_FSEL_ALT2>;
> > + };
>
> s/dpi_gpio4/dpi_gpio0
Yea, right, will fix.
cheers,
Gerd
^ permalink raw reply
* [PATCH V2 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang @ 2016-09-13 6:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <74367472.RYBpLsaIy1@wuerfel>
On 2016?09?08? 18:00, Arnd Bergmann wrote:
> On Thursday, September 8, 2016 4:06:01 PM CEST zhichang.yuan wrote:
>>>
>>>> +struct lpc_io_ops {
>>>> + unsigned int periosz;
>>>> + int (*lpc_iord)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr, unsigned char *buf,
>>>> + unsigned long dlen);
>>>> + int (*lpc_iowr)(struct hisilpc_dev *pdev, struct lpc_cycle_para *para,
>>>> + unsigned long ptaddr,
>>>> + const unsigned char *buf,
>>>> + unsigned long dlen);
>>>> +};
>>>
>>> The operations are not needed unless we also put the earlycon support
>>> in, so maybe leave them out from the first patch and only add them
>>> later (or drop the earlycon support if possible).
>>
>> Do you want to remove the struct lpc_io_ops member from struct hisilpc_dev??
>> I think we can not do that.
>>
>> These two functions are essential rd/wr operation for Hip06 LPC. They will be fallen into
>> when the upper layer drivers call their own IO in/out functions, such as serial_in/serial_out
>> for 8250 serial.
>>
>> I can define lpc_iord/lpc_iowr directly in struct hisilpc_dev and cancel the definition of
>> struct lpc_io_ops. In my original idea, several LPC cycle types will be supported. Each cycle
>> type has its specific ops. Now, only one cycle type is needed, the struct lpc_io_ops is not
>> meaningful.
>
> My point was that the indirect function calls are not needed at
> until patch four, so you can call the functions directly here,
> and make the logic for indirect function calls part of that
> later patch.
O. I think I got your meaning now.
In patch V2, the lpc_io_ops is not needed even if earlycon is supported. The early_in/early_out operation is
defined in hisi_lpc.c too, we can directly call the hisilpc_target_in/hisilpc_target_out to finish the LPC IO
operations.
At this moment, all the IO functions specific to the child devices of hip06 LPC have their own indirect call
method. This lpc_io_ops will be removed in V3.
>
> What are the other LPC cycle types that could be supported?
O. memory and firmware operations are supported too. But at this moment, we only use IO cycle.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply
* [PATCH V6 2/5] PCI/ACPI: Check platform specific ECAM quirks
From: Tomasz Nowicki @ 2016-09-13 6:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D76626.1050109@huawei.com>
Hi Liu,
On 13.09.2016 04:36, Dongdong Liu wrote:
> Hi Tomasz
>
> ? 2016/9/10 3:24, Tomasz Nowicki ??:
>> Some platforms may not be fully compliant with generic set of PCI config
>> accessors. For these cases we implement the way to overwrite CFG
>> accessors
>> set and configuration space range.
>>
>> In first place pci_mcfg_parse() saves machine's IDs and revision number
>> (these come from MCFG header) in order to match against known quirk
>> entries.
>> Then the algorithm traverses available quirk list (static array),
>> matches against <oem_id, oem_table_id, rev, domain, bus number range> and
>> returns custom PCI config ops and/or CFG resource structure.
>>
>> When adding new quirk there are two possibilities:
>> 1. Override default pci_generic_ecam_ops ops but CFG resource comes
>> from MCFG
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &foo_ops,
>> MCFG_RES_EMPTY },
>> 2. Override default pci_generic_ecam_ops ops and CFG resource. For
>> this case
>> it is also allowed get CFG resource from quirk entry w/o having it in
>> MCFG.
>> { "OEM_ID", "OEM_TABLE_ID", <REV>, <DOMAIN>, <BUS_NR>, &boo_ops,
>> DEFINE_RES_MEM(START, SIZE) },
>>
>> pci_generic_ecam_ops and MCFG entries will be used for platforms
>> free from quirks.
>>
>> Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>> drivers/acpi/pci_mcfg.c | 80
>> +++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 74 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index ffcc651..2b8acc7 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -32,6 +32,59 @@ struct mcfg_entry {
>> u8 bus_start;
>> u8 bus_end;
>> };
>> +struct mcfg_fixup {
>> + char oem_id[ACPI_OEM_ID_SIZE + 1];
>> + char oem_table_id[ACPI_OEM_TABLE_ID_SIZE + 1];
>> + u32 oem_revision;
>> + u16 seg;
>> + struct resource bus_range;
>> + struct pci_ecam_ops *ops;
>> + struct resource cfgres;
>> +};
>> +
>> +#define MCFG_DOM_ANY (-1)
>> +#define MCFG_BUS_RANGE(start, end) DEFINE_RES_NAMED((start), \
>> + ((end) - (start) + 1), \
>> + NULL, IORESOURCE_BUS)
>> +#define MCFG_BUS_ANY MCFG_BUS_RANGE(0x0, 0xff)
>> +#define MCFG_RES_EMPTY DEFINE_RES_NAMED(0, 0, NULL, 0)
>> +
>> +static struct mcfg_fixup mcfg_quirks[] = {
>> +/* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
>> +};
>> +
>> +static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> +static char mcfg_oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
>> +static u32 mcfg_oem_revision;
>> +
>> +static void pci_mcfg_match_quirks(struct acpi_pci_root *root,
>> + struct resource *cfgres,
>> + struct pci_ecam_ops **ecam_ops)
>> +{
>> + struct mcfg_fixup *f;
>> + int i;
>> +
>> + /*
>> + * First match against PCI topology <domain:bus> then use OEM ID,
>> OEM
>> + * table ID, and OEM revision from MCFG table standard header.
>> + */
>> + for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++,
>> f++) {
>> + if (f->seg == root->segment &&
>
> why not use MCFG_DOM_RANGE, I think MCFG_DOM_RANGE is better.
> if drop MCFG_DOM_RANGE, mcfg_quirks[] will be more complex.
>
> static struct mcfg_fixup mcfg_quirks[] = {
> /* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, cfgres, ops }, */
> #ifdef CONFIG_PCI_HOST_THUNDER_ECAM
> /* SoC pass1.x */
> { "CAVIUM", "THUNDERX", 2, 0, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 1, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 2, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 3, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 10, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 11, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 12, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> { "CAVIUM", "THUNDERX", 2, 13, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
> MCFG_RES_EMPTY},
> #endif
> .....
> };
>
> As PATCH v5 we only need define mcfg_quirks as below, It looks better.
> static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
> /* { OEM_ID, OEM_TABLE_ID, REV, DOMAIN, BUS_RANGE, pci_ops, init_hook
> }, */
> #ifdef CONFIG_PCI_HOST_THUNDER_PEM
> /* Pass2.0 */
> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(4, 9), MCFG_BUS_ANY, NULL,
> thunder_pem_cfg_init },
> { "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
> thunder_pem_cfg_init },
> #endif
> #ifdef CONFIG_PCI_HISI_ACPI
> { "HISI ", "HIP05 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> NULL, hisi_pcie_acpi_hip05_init},
> { "HISI ", "HIP06 ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
> NULL, hisi_pcie_acpi_hip06_init},
> { "HISI ", "HIP07 ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY,
> NULL, hisi_pcie_acpi_hip07_init},
> #endif
> };
Note this series disallow hisi_pcie_acpi_hip07_init() call. According to
the Bjorn suggestion I rework quirk code to override ops and CFG
resources only. Giving that I do not see the way to use MCFG_DOM_RANGE
macro. For HISI you would need to get CFG range for each possible case:
#ifdef CONFIG_PCI_HISI_ACPI
{ "HISI ", "HIP05 ", 0, 0, MCFG_BUS_ANY, &hisi_pcie_ops,
DEFINE_RES_MEM(start0, size0)},
{ "HISI ", "HIP05 ", 0, 1, MCFG_BUS_ANY, &hisi_pcie_ops,
DEFINE_RES_MEM(start1, size1)},
{ "HISI ", "HIP05 ", 0, 2, MCFG_BUS_ANY, &hisi_pcie_ops,
DEFINE_RES_MEM(start2, size2)},
{ "HISI ", "HIP05 ", 0, 3, MCFG_BUS_ANY, &hisi_pcie_ops,
DEFINE_RES_MEM(start3, size3)},
[...]
#endif
Indeed there are more entries here but you do not have to define the
same resource array in driver.
Thanks,
Tomasz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox