* [v16, 1/7] ARM64: dts: ls2080a: add device configuration node
From: Yangbo Lu @ 2016-11-09 3:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478661252-42439-1-git-send-email-yangbo.lu@nxp.com>
Add the dts node for device configuration unit that provides
general purpose configuration and status for the device.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
Acked-by: Scott Wood <oss@buserror.net>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
Changes for v5:
- Added this patch
Changes for v6:
- None
Changes for v7:
- None
Changes for v8:
- Added 'Acked-by: Scott Wood'
Changes for v9:
- None
Changes for v10:
- None
Changes for v11:
- None
Changes for v12:
- None
Changes for v13:
- None
Changes for v14:
- None
Changes for v15:
- None
Changes for v16:
- Added 'Acked-by: Arnd'
---
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 7f0dc13..d058e56 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -216,6 +216,12 @@
clocks = <&sysclk>;
};
+ dcfg: dcfg at 1e00000 {
+ compatible = "fsl,ls2080a-dcfg", "syscon";
+ reg = <0x0 0x1e00000 0x0 0x10000>;
+ little-endian;
+ };
+
serial0: serial at 21c0500 {
compatible = "fsl,ns16550", "ns16550a";
reg = <0x0 0x21c0500 0x0 0x100>;
--
2.1.0.27.g96db324
^ permalink raw reply related
* [v16, 0/7] Fix eSDHC host version register bug
From: Yangbo Lu @ 2016-11-09 3:14 UTC (permalink / raw)
To: linux-arm-kernel
This patchset is used to fix a host version register bug in the T4240-R1.0-R2.0
eSDHC controller. To match the SoC version and revision, 15 previous version
patchsets had tried many methods but all of them were rejected by reviewers.
Such as
- dts compatible method
- syscon method
- ifdef PPC method
- GUTS driver getting SVR method
Anrd suggested a soc_device_match method in v10, and this is the only available
method left now. This v11 patchset introduces the soc_device_match interface in
soc driver.
The first four patches of Yangbo are to add the GUTS driver. This is used to
register a soc device which contain soc version and revision information.
The other three patches introduce the soc_device_match method in soc driver
and apply it on esdhc driver to fix this bug.
---
Changes for v15:
- Dropped patch 'dt: bindings: update Freescale DCFG compatible'
since the work had been done by below patch on ShawnGuo's linux tree.
'dt-bindings: fsl: add LS1043A/LS1046A/LS2080A compatible for SCFG
and DCFG'
- Fixed error code issue in guts driver
Changes for v16:
- Dropped patch 'powerpc/fsl: move mpc85xx.h to include/linux/fsl'
- Added a bug-fix patch from Geert
---
Arnd Bergmann (1):
base: soc: introduce soc_device_match() interface
Geert Uytterhoeven (1):
base: soc: Check for NULL SoC device attributes
Yangbo Lu (5):
ARM64: dts: ls2080a: add device configuration node
dt: bindings: move guts devicetree doc out of powerpc directory
soc: fsl: add GUTS driver for QorIQ platforms
MAINTAINERS: add entry for Freescale SoC drivers
mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
.../bindings/{powerpc => soc}/fsl/guts.txt | 3 +
MAINTAINERS | 11 +-
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 6 +
drivers/base/Kconfig | 1 +
drivers/base/soc.c | 70 ++++++
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/sdhci-of-esdhc.c | 20 ++
drivers/soc/Kconfig | 3 +-
drivers/soc/fsl/Kconfig | 18 ++
drivers/soc/fsl/Makefile | 1 +
drivers/soc/fsl/guts.c | 236 +++++++++++++++++++++
include/linux/fsl/guts.h | 125 ++++++-----
include/linux/sys_soc.h | 3 +
13 files changed, 447 insertions(+), 51 deletions(-)
rename Documentation/devicetree/bindings/{powerpc => soc}/fsl/guts.txt (91%)
create mode 100644 drivers/soc/fsl/Kconfig
create mode 100644 drivers/soc/fsl/guts.c
--
2.1.0.27.g96db324
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe
From: Don Dutile @ 2016-11-09 2:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108163508.1bcae0c2@t450s.home>
On 11/08/2016 06:35 PM, Alex Williamson wrote:
> On Tue, 8 Nov 2016 21:29:22 +0100
> Christoffer Dall <christoffer.dall@linaro.org> wrote:
>
>> Hi Will,
>>
>> On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
>>> Hi all,
>>>
>>> I figured this was a reasonable post to piggy-back on for the LPC minutes
>>> relating to guest MSIs on arm64.
>>>
>>> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
>>>> We can always have QEMU reject hot-adding the device if the reserved
>>>> region overlaps existing guest RAM, but I don't even really see how we
>>>> advise users to give them a reasonable chance of avoiding that
>>>> possibility. Apparently there are also ARM platforms where MSI pages
>>>> cannot be remapped to support the previous programmable user/VM
>>>> address, is it even worthwhile to support those platforms? Does that
>>>> decision influence whether user programmable MSI reserved regions are
>>>> really a second class citizen to fixed reserved regions? I expect
>>>> we'll be talking about this tomorrow morning, but I certainly haven't
>>>> come up with any viable solutions to this. Thanks,
>>>
>>> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
>>> microconference. I presented some slides to illustrate some of the issues
>>> we're trying to solve:
>>>
>>> http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
>>>
>>> Punit took some notes (thanks!) on the etherpad here:
>>>
>>> https://etherpad.openstack.org/p/LPC2016_PCI
>>>
>>> although the discussion was pretty lively and jumped about, so I've had
>>> to go from memory where the notes didn't capture everything that was
>>> said.
>>>
>>> To summarise, arm64 platforms differ in their handling of MSIs when compared
>>> to x86:
>>>
>>> 1. The physical memory map is not standardised (Jon pointed out that
>>> this is something that was realised late on)
>>> 2. MSIs are usually treated the same as DMA writes, in that they must be
>>> mapped by the SMMU page tables so that they target a physical MSI
>>> doorbell
>>> 3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
>>> doorbell built into the PCI RC)
>>> 4. Platforms typically have some set of addresses that abort before
>>> reaching the SMMU (e.g. because the PCI identifies them as P2P).
>>>
>>> All of this means that userspace (QEMU) needs to identify the memory
>>> regions corresponding to points (3) and (4) and ensure that they are
>>> not allocated in the guest physical (IPA) space. For platforms that can
>>> remap the MSI doorbell as in (2), then some space also needs to be
>>> allocated for that.
>>>
>>> Rather than treat these as separate problems, a better interface is to
>>> tell userspace about a set of reserved regions, and have this include
>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>
>> Is my understanding correct, that you need to tell userspace about the
>> location of the doorbell (in the IOVA space) in case (2), because even
>> though the configuration of the device is handled by the (host) kernel
>> through trapping of the BARs, we have to avoid the VFIO user programming
>> the device to create other DMA transactions to this particular address,
>> since that will obviously conflict and either not produce the desired
>> DMA transactions or result in unintended weird interrupts?
>
> Correct, if the MSI doorbell IOVA range overlaps RAM in the VM, then
> it's potentially a DMA target and we'll get bogus data on DMA read from
> the device, and lose data and potentially trigger spurious interrupts on
> DMA write from the device. Thanks,
>
> Alex
>
That's b/c the MSI doorbells are not positioned *above* the SMMU, i.e.,
they address match before the SMMU checks are done. if
all DMA addrs had to go through SMMU first, then the DMA access could
be ignored/rejected.
For bare-metal, memory can't be put in the same place as MSI addrs, or
DMA could never reach it. So, only a virt issue, unless the VMs mem address
range mimic the host layout.
- Don
^ permalink raw reply
* [PATCH] arm/arm64: KVM: clean up useless code in kvm_timer_enable
From: Longpeng(Mike) @ 2016-11-09 2:50 UTC (permalink / raw)
To: linux-arm-kernel
1) Since commit:41a54482 changed timer enabled variable to per-vcpu,
the correlative comment in kvm_timer_enable is useless now.
2) After the kvm module init successfully, the timecounter is always
non-null, so we can remove the checking of timercounter.
Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
virt/kvm/arm/arch_timer.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 27a1f63..17b8fa5 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -498,17 +498,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)
if (ret)
return ret;
-
- /*
- * There is a potential race here between VCPUs starting for the first
- * time, which may be enabling the timer multiple times. That doesn't
- * hurt though, because we're just setting a variable to the same
- * variable that it already was. The important thing is that all
- * VCPUs have the enabled variable set, before entering the guest, if
- * the arch timers are enabled.
- */
- if (timecounter)
- timer->enabled = 1;
+ timer->enabled = 1;
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH] ARM: dts: imx7d-pinfunc: fix UART pinmux defines
From: Stefan Agner @ 2016-11-09 2:44 UTC (permalink / raw)
To: linux-arm-kernel
The UART pinmux defines for the pins which are part of the LPSR
pinmux controller are wrong: Output signals configure the input
sel value and the pinmux defines allow not to distinguish between
DCE/DTE mode. Follow the usual pattern using DTE/DCE as part of
the define to denote the two UART configuration options.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
There is a similar patch downstream named:
"MLK-10662: arm: imx7d-pinfunc: fix uart input sel option value"
Tested using UART5 on a Colibri iMX7.
--
Stefan
arch/arm/boot/dts/imx7d-pinfunc.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx7d-pinfunc.h b/arch/arm/boot/dts/imx7d-pinfunc.h
index 3f9f0d9..7bc3c00 100644
--- a/arch/arm/boot/dts/imx7d-pinfunc.h
+++ b/arch/arm/boot/dts/imx7d-pinfunc.h
@@ -43,26 +43,30 @@
#define MX7D_PAD_GPIO1_IO04__GPIO1_IO4 0x0010 0x0040 0x0000 0x0 0x0
#define MX7D_PAD_GPIO1_IO04__USB_OTG1_OC 0x0010 0x0040 0x072C 0x1 0x1
#define MX7D_PAD_GPIO1_IO04__FLEXTIMER1_CH4 0x0010 0x0040 0x0594 0x2 0x1
-#define MX7D_PAD_GPIO1_IO04__UART5_CTS_B 0x0010 0x0040 0x0710 0x3 0x4
+#define MX7D_PAD_GPIO1_IO04__UART5_DCE_CTS 0x0010 0x0040 0x0000 0x3 0x0
+#define MX7D_PAD_GPIO1_IO04__UART5_DTE_RTS 0x0010 0x0040 0x0710 0x3 0x4
#define MX7D_PAD_GPIO1_IO04__I2C1_SCL 0x0010 0x0040 0x05D4 0x4 0x2
#define MX7D_PAD_GPIO1_IO04__OBSERVE3_OUT 0x0010 0x0040 0x0000 0x6 0x0
#define MX7D_PAD_GPIO1_IO05__GPIO1_IO5 0x0014 0x0044 0x0000 0x0 0x0
#define MX7D_PAD_GPIO1_IO05__USB_OTG1_PWR 0x0014 0x0044 0x0000 0x1 0x0
#define MX7D_PAD_GPIO1_IO05__FLEXTIMER1_CH5 0x0014 0x0044 0x0598 0x2 0x1
-#define MX7D_PAD_GPIO1_IO05__UART5_RTS_B 0x0014 0x0044 0x0710 0x3 0x5
+#define MX7D_PAD_GPIO1_IO05__UART5_DCE_RTS 0x0014 0x0044 0x0710 0x3 0x5
+#define MX7D_PAD_GPIO1_IO05__UART5_DTE_CTS 0x0014 0x0044 0x0000 0x3 0x0
#define MX7D_PAD_GPIO1_IO05__I2C1_SDA 0x0014 0x0044 0x05D8 0x4 0x2
#define MX7D_PAD_GPIO1_IO05__OBSERVE4_OUT 0x0014 0x0044 0x0000 0x6 0x0
#define MX7D_PAD_GPIO1_IO06__GPIO1_IO6 0x0018 0x0048 0x0000 0x0 0x0
#define MX7D_PAD_GPIO1_IO06__USB_OTG2_OC 0x0018 0x0048 0x0728 0x1 0x1
#define MX7D_PAD_GPIO1_IO06__FLEXTIMER1_CH6 0x0018 0x0048 0x059C 0x2 0x1
-#define MX7D_PAD_GPIO1_IO06__UART5_RX_DATA 0x0018 0x0048 0x0714 0x3 0x4
+#define MX7D_PAD_GPIO1_IO06__UART5_DCE_RX 0x0018 0x0048 0x0714 0x3 0x4
+#define MX7D_PAD_GPIO1_IO06__UART5_DTE_TX 0x0018 0x0048 0x0000 0x3 0x0
#define MX7D_PAD_GPIO1_IO06__I2C2_SCL 0x0018 0x0048 0x05DC 0x4 0x2
#define MX7D_PAD_GPIO1_IO06__CCM_WAIT 0x0018 0x0048 0x0000 0x5 0x0
#define MX7D_PAD_GPIO1_IO06__KPP_ROW4 0x0018 0x0048 0x0624 0x6 0x1
#define MX7D_PAD_GPIO1_IO07__GPIO1_IO7 0x001C 0x004C 0x0000 0x0 0x0
#define MX7D_PAD_GPIO1_IO07__USB_OTG2_PWR 0x001C 0x004C 0x0000 0x1 0x0
#define MX7D_PAD_GPIO1_IO07__FLEXTIMER1_CH7 0x001C 0x004C 0x05A0 0x2 0x1
-#define MX7D_PAD_GPIO1_IO07__UART5_TX_DATA 0x001C 0x004C 0x0714 0x3 0x5
+#define MX7D_PAD_GPIO1_IO07__UART5_DCE_TX 0x001C 0x004C 0x0000 0x3 0x0
+#define MX7D_PAD_GPIO1_IO07__UART5_DTE_RX 0x001C 0x004C 0x0714 0x3 0x5
#define MX7D_PAD_GPIO1_IO07__I2C2_SDA 0x001C 0x004C 0x05E0 0x4 0x2
#define MX7D_PAD_GPIO1_IO07__CCM_STOP 0x001C 0x004C 0x0000 0x5 0x0
#define MX7D_PAD_GPIO1_IO07__KPP_COL4 0x001C 0x004C 0x0604 0x6 0x1
--
2.10.2
^ permalink raw reply related
* RE: [PATCH v4] PM/devfreq: add suspend frequency support
From: MyungJoo Ham @ 2016-11-09 1:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CGME20161108091840epcas3p21af60a1b0a4867306366633427567dc3@epcas3p2.samsung.com>
> Add suspend frequency support and if needed set it to
> the frequency obtained from the suspend opp (can be defined
> using opp-v2 bindings and is optional).
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> Changes in v2:
> - use update_devfreq() instead devfreq_update_status()
> Changes in v3:
> - fix build error
> Changes in v4:
> - move dev_pm_opp_get_suspend_opp() to devfreq_add_device()
>
> drivers/devfreq/devfreq.c | 15 +++++++++++++--
> drivers/devfreq/governor_simpleondemand.c | 9 +++++++++
> include/linux/devfreq.h | 9 +++++++++
> 3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index bf3ea76..d9d56e1 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -363,7 +363,10 @@ void devfreq_monitor_suspend(struct devfreq *devfreq)
> mutex_unlock(&devfreq->lock);
> return;
> }
> -
> + if (devfreq->suspend_freq) {
> + update_devfreq(devfreq);
> + devfreq->suspend_flag = true;
+ } else {
+ devfreq_update_status(devfreq, devfreq->previous_freq);
+ }
- devfreq_update_status(devfreq, devfreq->previous_freq);
update_devfreq() calls devfreq_update_status.
So let's call it only when update_devfreq is not called.
> devfreq->stop_polling = true;
> mutex_unlock(&devfreq->lock);
> @@ -394,7 +397,8 @@ void devfreq_monitor_resume(struct devfreq *devfreq)
>
> devfreq->last_stat_updated = jiffies;
> devfreq->stop_polling = false;
> -
> + if (devfreq->suspend_freq)
> + devfreq->suspend_flag = false;
> if (devfreq->profile->get_cur_freq &&
> !devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
> devfreq->previous_freq = freq;
> @@ -528,6 +532,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> struct devfreq *devfreq;
> struct devfreq_governor *governor;
> int err = 0;
> + struct dev_pm_opp *suspend_opp;
>
> if (!dev || !profile || !governor_name) {
> dev_err(dev, "%s: Invalid parameters.\n", __func__);
> @@ -563,6 +568,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
> devfreq->data = data;
> devfreq->nb.notifier_call = devfreq_notifier_call;
>
> + rcu_read_lock();
> + suspend_opp = dev_pm_opp_get_suspend_opp(dev);
> + if (suspend_opp)
> + devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
> + rcu_read_unlock();
> +
> if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
> mutex_unlock(&devfreq->lock);
> devfreq_set_freq_table(devfreq);
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index ae72ba5..84b3ce1 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
> struct devfreq_simple_ondemand_data *data = df->data;
> unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>
> + /*
> + * if devfreq in suspend status and have suspend_freq,
> + * the frequency need to set to suspend_freq
> + */
> + if (df->suspend_flag) {
> + *freq = df->suspend_freq;
> + return 0;
> + }
> +
If this is because of the case when devfreq_simple_ondemand_func() is called
before cancel_delayed_work_sync() in devfreq_monitor_suspend() and
after update_devfreq() in devfreq_monitor_suspend(),
you can use devfreq->stop_polling (and move devfreq->stop_polling = true; a bit earlier).
You do not need to introduce yet another variable, suspend_flag, in the devfreq struct.
The semantics of "stop_polling" fits the purpose well enough.
> err = devfreq_update_stats(df);
> if (err)
> return err;
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 2de4e2e..c463ae1 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -172,6 +172,7 @@ struct devfreq {
> struct delayed_work work;
>
> unsigned long previous_freq;
> + unsigned long suspend_freq;
> struct devfreq_dev_status last_status;
>
> void *data; /* private data for governors */
> @@ -179,6 +180,7 @@ struct devfreq {
> unsigned long min_freq;
> unsigned long max_freq;
> bool stop_polling;
> + bool suspend_flag;
>
> /* information for device frequency transition */
> unsigned int total_trans;
> @@ -214,6 +216,8 @@ extern int devfreq_resume_device(struct devfreq *devfreq);
> /* Helper functions for devfreq user device driver with OPP. */
> extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
> unsigned long *freq, u32 flags);
> +extern void devfreq_opp_get_suspend_opp(struct device *dev,
> + struct devfreq *devfreq);
No more required.
> extern int devfreq_register_opp_notifier(struct device *dev,
> struct devfreq *devfreq);
> extern int devfreq_unregister_opp_notifier(struct device *dev,
> @@ -348,6 +352,11 @@ static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
> return ERR_PTR(-EINVAL);
> }
>
> +static inline void devfreq_opp_get_suspend_opp(struct device *dev,
> + struct devfreq *devfreq)
> +{
> +}
> +
No more required.
> static inline int devfreq_register_opp_notifier(struct device *dev,
> struct devfreq *devfreq)
> {
> --
> 2.6.6
>
>
>
^ permalink raw reply
* [PATCH v2 0/2] arm64: fix the bugs found in the hugetlb test
From: Huang Shijie @ 2016-11-09 1:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108163642.GE20591@arm.com>
Hi Will & Catalin,
On Tue, Nov 08, 2016 at 04:36:43PM +0000, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 02:09:09PM +0000, Catalin Marinas wrote:
> > On Tue, Nov 08, 2016 at 01:44:37PM +0800, Huang Shijie wrote:
> > > (3) The test result in the Softiron and Juno-r1 boards:
> > >
> > > This detail test result shows below (both the "make func" & "make stress"):
> > >
> > > 4KB granule:
> > >
> > > 1.1) PTE + Contiguous bit : 4K x 16 = 64K (per huge page size)
> > > Test result : PASS
> > >
> > > 1.2) PMD : 2M x 1 = 2M (per huge page size)
> > > Test result : PASS
> > >
> > > 1.3) PMD + Contiguous bit : 2M x 16 = 32M (per huge page size)
> > > Test result : PASS
> > >
> > > 64KB granule:
> > >
> > > 3.1) PTE + Contiguous bit : 64K x 32 = 2M (per huge page size)
> > > Test result : PASS
> > >
> > > 3.2) PMD + Contiguous bit : 512M x 32 = 16G (per huge page size)
> > > Test result : no hardware to support this test
> >
> > Don't we have support for single (non-contiguous) PMD huge page with 64K
> > pages (512M per huge page)? I gave it a try and it seems to work (though
> > without your patches applied ;)).
Yes, it should be okay. This patch set does not affect the the single
PMD huge page.
> >
> > > Huang Shijie (2):
> > > arm64: hugetlb: remove the wrong pmd check in find_num_contig()
> > > arm64: hugetlb: fix the wrong address for several functions
> >
> > For these patches:
> >
> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Thanks you, Catalin.
> >
> > I'm not sure whether Will plans to push them into 4.9. AFAICT, the
> > contiguous huge pages never worked properly, so we may not count it as a
> > regression but a new feature. If Will doesn't take them, I'll queue the
> > patches for 4.10.
>
> Right, given that it's never worked and the failure only seems to crop up
> in synthetic testing, I think you can queue these for 4.10.
Okay. Thank for queue them for 4.10.
Thanks
Huang Shijie
^ permalink raw reply
* [PATCH v2 1/3] ARM: dts: imx6qdl-apalis: Do not rely on DDC I2C bus bitbang for HDMI
From: Stefan Agner @ 2016-11-09 0:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108173315.GA16082@Sanchayan-Arch.localdomain>
On 2016-11-08 09:33, maitysanchayan at gmail.com wrote:
> Hello Shawn,
>
> On 16-10-22 15:43:04, Vladimir Zapolskiy wrote:
>> Hi Shawn,
>>
>> On 10/22/2016 06:25 AM, Shawn Guo wrote:
>> > On Mon, Sep 19, 2016 at 10:41:51AM +0530, Sanchayan Maity wrote:
>> > > Remove the use of DDC I2C bus bitbang to support reading of EDID
>> > > and rely on support from internal HDMI I2C master controller instead.
>> > > As a result remove the device tree property ddc-i2c-bus.
>> > >
>> > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>> >
>> > I think that the dw-hdmi i2c support [1] is a prerequisite of this
>> > patch. I do not see it lands on v4.9-rc1. Or am I missing something?
>> >
>> > Shawn
>> >
>> > [1] https://patchwork.kernel.org/patch/9296883/
>> >
>>
>> I'm adding Philipp to Cc, since he is the last one who tested the change
>> and helped me to push the change to the mainline:
>>
>> https://lists.freedesktop.org/archives/dri-devel/2016-September/118569.html
>>
>> The problem is that there is no official DW HDMI bridge maintainer, may be
>> you can review the change, and if you find it satisfactory push it through
>> ARM/iMX tree.
>
> Shawn, is it okay if that patch goes through your ARM/iMX tree?
I don't think it makes sense that the DRM bridge changes go through
Shawn's tree. Dave should merge Philipps pull request...
--
Stefan
^ permalink raw reply
* [PATCH] ARM: socfpga_defconfig: enable FS configs to support Angstrom filesystem
From: Dinh Nguyen @ 2016-11-09 0:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2911351.Izg0zkPBzd@wuerfel>
On Tue, Nov 8, 2016 at 4:57 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, November 8, 2016 4:47:50 PM CET Dinh Nguyen wrote:
>> Enables:
>>
>> CONFIG_AUTOFS4_FS=y
>> CONFIG_NFSD=y
>> CONFIG_NFS_V3_ACL=y
>> CONFIG_NFSD_V3_ACL=y
>> CONFIG_NFSD_V4=y
>>
>> Signed-off-by: Matthew Gerlach <mgerlach@opensource.altera.com>
>> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>>
>
> No objections to the patch, but the changelog should not state what
> the patch clearly shows already but instead it should explain why
> you do this.
>
> The subject line mentions "to support Angstrom filesystem", but
> it's not clear (at least not to me) what that filesystem is.
>
Thanks for the review Arnd. I'll update with a better changelog message.
Dinh
^ permalink raw reply
* [PATCH v7 1/3] input: touchscreen: TOUCHSCREEN_SUN4I depends on !SUN4I_GPADC
From: Dmitry Torokhov @ 2016-11-09 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161103101601.24529-2-quentin.schulz@free-electrons.com>
On Thu, Nov 03, 2016 at 11:15:59AM +0100, Quentin Schulz wrote:
> SUN4I_GPADC and TOUCHSCREEN_SUN4I are incompatible (both are drivers
> for Allwinner SoCs' ADC). This makes sure TOUCHSCREEN_SUN4I isn't
> enabled while SUN4I_GPADC is enabled.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Please merge with SUN4I_GPADC driver patch.
> ---
>
> added in v7
>
> drivers/input/touchscreen/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index efca013..c618cb9 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1120,6 +1120,7 @@ config TOUCHSCREEN_SUN4I
> depends on ARCH_SUNXI || COMPILE_TEST
> depends on HWMON
> depends on THERMAL || !THERMAL_OF
> + depends on !SUN4I_GPADC
> help
> This selects support for the resistive touchscreen controller
> found on Allwinner sunxi SoCs.
> --
> 2.9.3
>
--
Dmitry
^ permalink raw reply
* [PATCH v15 2/4] reset: mediatek: Add MT2701 reset driver
From: Stephen Boyd @ 2016-11-09 0:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478245388-1412-3-git-send-email-erin.lo@mediatek.com>
On 11/04, Erin Lo wrote:
> From: Shunli Wang <shunli.wang@mediatek.com>
>
> In infrasys and perifsys, there are many reset
> control bits for kinds of modules. These bits are
> used as actual reset controllers to be registered
> into kernel's generic reset controller framework.
>
> Signed-off-by: Shunli Wang <shunli.wang@mediatek.com>
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> Tested-by: John Crispin <blogic@openwrt.org>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v15 1/4] clk: mediatek: Add MT2701 clock support
From: Stephen Boyd @ 2016-11-09 0:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478245388-1412-2-git-send-email-erin.lo@mediatek.com>
On 11/04, Erin Lo wrote:
> From: Shunli Wang <shunli.wang@mediatek.com>
>
> Add MT2701 clock support, include topckgen, apmixedsys,
> infracfg, pericfg and subsystem clocks.
>
> Signed-off-by: Shunli Wang <shunli.wang@mediatek.com>
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> Tested-by: John Crispin <blogic@openwrt.org>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH v4 1/5] arm64: perf: Basic uncore counter support for Cavium ThunderX SOC
From: Will Deacon @ 2016-11-08 23:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73173d6ad2430eead5e9da40564a90a60961b6d9.1477741719.git.jglauber@cavium.com>
Hi Jan,
Thanks for posting an updated series. I have a few minor comments, which
we can hopefully address in time for 4.10.
Also, have you run the perf fuzzer with this series applied?
https://github.com/deater/perf_event_tests
(build the tests and look under the fuzzer/ directory for the tool)
On Sat, Oct 29, 2016 at 01:55:29PM +0200, Jan Glauber wrote:
> Provide "uncore" facilities for different non-CPU performance
> counter units.
>
> The uncore PMUs can be found under /sys/bus/event_source/devices.
> All counters are exported via sysfs in the corresponding events
> files under the PMU directory so the perf tool can list the event names.
>
> There are some points that are special in this implementation:
>
> 1) The PMU detection relies on PCI device detection. If a
> matching PCI device is found the PMU is created. The code can deal
> with multiple units of the same type, e.g. more than one memory
> controller.
>
> 2) Counters are summarized across different units of the same type
> on one NUMA node but not across NUMA nodes.
> For instance L2C TAD 0..7 are presented as a single counter
> (adding the values from TAD 0 to 7). Although losing the ability
> to read a single value the merged values are easier to use.
>
> 3) The counters are not CPU related. A random CPU is picked regardless
> of the NUMA node. There is a small performance penalty for accessing
> counters on a remote note but reading a performance counter is a
> slow operation anyway.
>
> Signed-off-by: Jan Glauber <jglauber@cavium.com>
> ---
> drivers/perf/Kconfig | 13 ++
> drivers/perf/Makefile | 1 +
> drivers/perf/uncore/Makefile | 1 +
> drivers/perf/uncore/uncore_cavium.c | 351 ++++++++++++++++++++++++++++++++++++
> drivers/perf/uncore/uncore_cavium.h | 71 ++++++++
We already have "uncore" PMUs under drivers/perf, so I'd prefer that we
renamed this a bit to reflect better what's going on. How about:
drivers/perf/cavium/
and then
drivers/perf/cavium/uncore_thunder.[ch]
?
> include/linux/cpuhotplug.h | 1 +
> 6 files changed, 438 insertions(+)
> create mode 100644 drivers/perf/uncore/Makefile
> create mode 100644 drivers/perf/uncore/uncore_cavium.c
> create mode 100644 drivers/perf/uncore/uncore_cavium.h
>
> diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
> index 4d5c5f9..3266c87 100644
> --- a/drivers/perf/Kconfig
> +++ b/drivers/perf/Kconfig
> @@ -19,4 +19,17 @@ config XGENE_PMU
> help
> Say y if you want to use APM X-Gene SoC performance monitors.
>
> +config UNCORE_PMU
> + bool
This isn't needed.
> +
> +config UNCORE_PMU_CAVIUM
> + depends on PERF_EVENTS && NUMA && ARM64
> + bool "Cavium uncore PMU support"
Please mentioned Thunder somewhere, since that's the SoC being supported.
> + select UNCORE_PMU
> + default y
> + help
> + Say y if you want to access performance counters of subsystems
> + on a Cavium SOC like cache controller, memory controller or
> + processor interconnect.
> +
> endmenu
> diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
> index b116e98..d6c02c9 100644
> --- a/drivers/perf/Makefile
> +++ b/drivers/perf/Makefile
> @@ -1,2 +1,3 @@
> obj-$(CONFIG_ARM_PMU) += arm_pmu.o
> obj-$(CONFIG_XGENE_PMU) += xgene_pmu.o
> +obj-y += uncore/
> diff --git a/drivers/perf/uncore/Makefile b/drivers/perf/uncore/Makefile
> new file mode 100644
> index 0000000..6130e18
> --- /dev/null
> +++ b/drivers/perf/uncore/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_UNCORE_PMU_CAVIUM) += uncore_cavium.o
> diff --git a/drivers/perf/uncore/uncore_cavium.c b/drivers/perf/uncore/uncore_cavium.c
> new file mode 100644
> index 0000000..a7b4277
> --- /dev/null
> +++ b/drivers/perf/uncore/uncore_cavium.c
> @@ -0,0 +1,351 @@
> +/*
> + * Cavium Thunder uncore PMU support.
> + *
> + * Copyright (C) 2015,2016 Cavium Inc.
> + * Author: Jan Glauber <jan.glauber@cavium.com>
> + */
> +
> +#include <linux/cpufeature.h>
> +#include <linux/numa.h>
> +#include <linux/slab.h>
> +
> +#include "uncore_cavium.h"
> +
> +/*
> + * Some notes about the various counters supported by this "uncore" PMU
> + * and the design:
> + *
> + * All counters are 64 bit long.
> + * There are no overflow interrupts.
> + * Counters are summarized per node/socket.
> + * Most devices appear as separate PCI devices per socket with the exception
> + * of OCX TLK which appears as one PCI device per socket and contains several
> + * units with counters that are merged.
> + * Some counters are selected via a control register (L2C TAD) and read by
> + * a number of counter registers, others (L2C CBC, LMC & OCX TLK) have
> + * one dedicated counter per event.
> + * Some counters are not stoppable (L2C CBC & LMC).
> + * Some counters are read-only (LMC).
> + * All counters belong to PCI devices, the devices may have additional
> + * drivers but we assume we are the only user of the counter registers.
> + * We map the whole PCI BAR so we must be careful to forbid access to
> + * addresses that contain neither counters nor counter control registers.
> + */
> +
> +void thunder_uncore_read(struct perf_event *event)
> +{
Rather than have a bunch of global symbols that are called from the
individual drivers, why don't you pass a struct of function pointers to
their respective init functions and keep the internals private?
> + struct thunder_uncore *uncore = to_uncore(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + struct thunder_uncore_node *node;
> + struct thunder_uncore_unit *unit;
> + u64 prev, delta, new = 0;
> +
> + node = get_node(hwc->config, uncore);
> +
> + /* read counter values from all units on the node */
> + list_for_each_entry(unit, &node->unit_list, entry)
> + new += readq(hwc->event_base + unit->map);
> +
> + prev = local64_read(&hwc->prev_count);
> + local64_set(&hwc->prev_count, new);
> + delta = new - prev;
> + local64_add(delta, &event->count);
> +}
> +
> +int thunder_uncore_add(struct perf_event *event, int flags, u64 config_base,
> + u64 event_base)
> +{
> + struct thunder_uncore *uncore = to_uncore(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + struct thunder_uncore_node *node;
> + int id;
> +
> + node = get_node(hwc->config, uncore);
> + id = get_id(hwc->config);
> +
> + if (!cmpxchg(&node->events[id], NULL, event))
> + hwc->idx = id;
Does this need to be a full-fat cmpxchg? Who are you racing with?
> +
> + if (hwc->idx == -1)
> + return -EBUSY;
This would be much clearer as an else statement after the cmpxchg.
> +
> + hwc->config_base = config_base;
> + hwc->event_base = event_base;
> + hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
> +
> + if (flags & PERF_EF_START)
> + uncore->pmu.start(event, PERF_EF_RELOAD);
> +
> + return 0;
> +}
> +
> +void thunder_uncore_del(struct perf_event *event, int flags)
> +{
> + struct thunder_uncore *uncore = to_uncore(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + struct thunder_uncore_node *node;
> + int i;
> +
> + event->pmu->stop(event, PERF_EF_UPDATE);
> +
> + /*
> + * For programmable counters we need to check where we installed it.
> + * To keep this function generic always test the more complicated
> + * case (free running counters won't need the loop).
> + */
> + node = get_node(hwc->config, uncore);
> + for (i = 0; i < node->num_counters; i++) {
> + if (cmpxchg(&node->events[i], event, NULL) == event)
> + break;
> + }
> + hwc->idx = -1;
> +}
> +
> +void thunder_uncore_start(struct perf_event *event, int flags)
> +{
> + struct thunder_uncore *uncore = to_uncore(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + struct thunder_uncore_node *node;
> + struct thunder_uncore_unit *unit;
> + u64 new = 0;
> +
> + /* read counter values from all units on the node */
> + node = get_node(hwc->config, uncore);
> + list_for_each_entry(unit, &node->unit_list, entry)
> + new += readq(hwc->event_base + unit->map);
> + local64_set(&hwc->prev_count, new);
> +
> + hwc->state = 0;
> + perf_event_update_userpage(event);
> +}
> +
> +void thunder_uncore_stop(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> +
> + hwc->state |= PERF_HES_STOPPED;
> +
> + if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
> + thunder_uncore_read(event);
> + hwc->state |= PERF_HES_UPTODATE;
> + }
> +}
> +
> +int thunder_uncore_event_init(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct thunder_uncore_node *node;
> + struct thunder_uncore *uncore;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /* we do not support sampling */
> + if (is_sampling_event(event))
> + return -EINVAL;
> +
> + /* counters do not have these bits */
> + if (event->attr.exclude_user ||
> + event->attr.exclude_kernel ||
> + event->attr.exclude_host ||
> + event->attr.exclude_guest ||
> + event->attr.exclude_hv ||
> + event->attr.exclude_idle)
> + return -EINVAL;
> +
> + uncore = to_uncore(event->pmu);
> + if (!uncore)
> + return -ENODEV;
> + if (!uncore->event_valid(event->attr.config & UNCORE_EVENT_ID_MASK))
> + return -EINVAL;
> +
> + /* check NUMA node */
> + node = get_node(event->attr.config, uncore);
> + if (!node) {
> + pr_debug("Invalid NUMA node selected\n");
> + return -EINVAL;
> + }
> +
> + hwc->config = event->attr.config;
> + hwc->idx = -1;
> + return 0;
> +}
> +
> +static ssize_t thunder_uncore_attr_show_cpumask(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct pmu *pmu = dev_get_drvdata(dev);
> + struct thunder_uncore *uncore =
> + container_of(pmu, struct thunder_uncore, pmu);
> +
> + return cpumap_print_to_pagebuf(true, buf, &uncore->active_mask);
> +}
> +static DEVICE_ATTR(cpumask, S_IRUGO, thunder_uncore_attr_show_cpumask, NULL);
> +
> +static struct attribute *thunder_uncore_attrs[] = {
> + &dev_attr_cpumask.attr,
> + NULL,
> +};
> +
> +struct attribute_group thunder_uncore_attr_group = {
> + .attrs = thunder_uncore_attrs,
> +};
> +
> +ssize_t thunder_events_sysfs_show(struct device *dev,
> + struct device_attribute *attr,
> + char *page)
> +{
> + struct perf_pmu_events_attr *pmu_attr =
> + container_of(attr, struct perf_pmu_events_attr, attr);
> +
> + if (pmu_attr->event_str)
> + return sprintf(page, "%s", pmu_attr->event_str);
> +
> + return 0;
> +}
> +
> +/* node attribute depending on number of NUMA nodes */
> +static ssize_t node_show(struct device *dev, struct device_attribute *attr,
> + char *page)
> +{
> + if (NODES_SHIFT)
> + return sprintf(page, "config:16-%d\n", 16 + NODES_SHIFT - 1);
If NODES_SHIFT is 1, you'll end up with "config:16-16", which might confuse
userspace.
> + else
> + return sprintf(page, "config:16\n");
> +}
> +
> +struct device_attribute format_attr_node = __ATTR_RO(node);
> +
> +/*
> + * Thunder uncore events are independent from CPUs. Provide a cpumask
> + * nevertheless to prevent perf from adding the event per-cpu and just
> + * set the mask to one online CPU. Use the same cpumask for all uncore
> + * devices.
> + *
> + * There is a performance penalty for accessing a device from a CPU on
> + * another socket, but we do not care (yet).
> + */
> +static int thunder_uncore_offline_cpu(unsigned int old_cpu, struct hlist_node *node)
> +{
> + struct thunder_uncore *uncore = hlist_entry_safe(node, struct thunder_uncore, node);
Why _safe?
> + int new_cpu;
> +
> + if (!cpumask_test_and_clear_cpu(old_cpu, &uncore->active_mask))
> + return 0;
> + new_cpu = cpumask_any_but(cpu_online_mask, old_cpu);
> + if (new_cpu >= nr_cpu_ids)
> + return 0;
> + perf_pmu_migrate_context(&uncore->pmu, old_cpu, new_cpu);
> + cpumask_set_cpu(new_cpu, &uncore->active_mask);
> + return 0;
> +}
> +
> +static struct thunder_uncore_node * __init alloc_node(struct thunder_uncore *uncore,
> + int node_id, int counters)
> +{
> + struct thunder_uncore_node *node;
> +
> + node = kzalloc(sizeof(*node), GFP_KERNEL);
> + if (!node)
> + return NULL;
> + node->num_counters = counters;
> + INIT_LIST_HEAD(&node->unit_list);
> + return node;
> +}
> +
> +int __init thunder_uncore_setup(struct thunder_uncore *uncore, int device_id,
> + struct pmu *pmu, int counters)
> +{
> + unsigned int vendor_id = PCI_VENDOR_ID_CAVIUM;
> + struct thunder_uncore_unit *unit, *tmp;
> + struct thunder_uncore_node *node;
> + struct pci_dev *pdev = NULL;
> + int ret, node_id, found = 0;
> +
> + /* detect PCI devices */
> + while ((pdev = pci_get_device(vendor_id, device_id, pdev))) {
Redundant brackets?
> + if (!pdev)
> + break;
Redundant check?
> + node_id = dev_to_node(&pdev->dev);
> +
> + /* allocate node if necessary */
> + if (!uncore->nodes[node_id])
> + uncore->nodes[node_id] = alloc_node(uncore, node_id, counters);
> +
> + node = uncore->nodes[node_id];
> + if (!node) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + unit = kzalloc(sizeof(*unit), GFP_KERNEL);
> + if (!unit) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + unit->pdev = pdev;
> + unit->map = ioremap(pci_resource_start(pdev, 0),
> + pci_resource_len(pdev, 0));
> + list_add(&unit->entry, &node->unit_list);
> + node->nr_units++;
> + found++;
> + }
> +
> + if (!found)
> + return -ENODEV;
> +
> + cpuhp_state_add_instance_nocalls(CPUHP_AP_UNCORE_CAVIUM_ONLINE,
> + &uncore->node);
> +
> + /*
> + * perf PMU is CPU dependent in difference to our uncore devices.
> + * Just pick a CPU and migrate away if it goes offline.
> + */
> + cpumask_set_cpu(smp_processor_id(), &uncore->active_mask);
> +
> + uncore->pmu = *pmu;
> + ret = perf_pmu_register(&uncore->pmu, uncore->pmu.name, -1);
> + if (ret)
> + goto fail;
> +
> + return 0;
> +
> +fail:
> + node_id = 0;
> + while (uncore->nodes[node_id]) {
> + node = uncore->nodes[node_id];
> +
> + list_for_each_entry_safe(unit, tmp, &node->unit_list, entry) {
Why do you need the _safe variant?
Will
^ permalink raw reply
* Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II))
From: Alex Williamson @ 2016-11-08 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108202922.GC15676@cbox>
On Tue, 8 Nov 2016 21:29:22 +0100
Christoffer Dall <christoffer.dall@linaro.org> wrote:
> Hi Will,
>
> On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
> > Hi all,
> >
> > I figured this was a reasonable post to piggy-back on for the LPC minutes
> > relating to guest MSIs on arm64.
> >
> > On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
> > > We can always have QEMU reject hot-adding the device if the reserved
> > > region overlaps existing guest RAM, but I don't even really see how we
> > > advise users to give them a reasonable chance of avoiding that
> > > possibility. Apparently there are also ARM platforms where MSI pages
> > > cannot be remapped to support the previous programmable user/VM
> > > address, is it even worthwhile to support those platforms? Does that
> > > decision influence whether user programmable MSI reserved regions are
> > > really a second class citizen to fixed reserved regions? I expect
> > > we'll be talking about this tomorrow morning, but I certainly haven't
> > > come up with any viable solutions to this. Thanks,
> >
> > At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> > microconference. I presented some slides to illustrate some of the issues
> > we're trying to solve:
> >
> > http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
> >
> > Punit took some notes (thanks!) on the etherpad here:
> >
> > https://etherpad.openstack.org/p/LPC2016_PCI
> >
> > although the discussion was pretty lively and jumped about, so I've had
> > to go from memory where the notes didn't capture everything that was
> > said.
> >
> > To summarise, arm64 platforms differ in their handling of MSIs when compared
> > to x86:
> >
> > 1. The physical memory map is not standardised (Jon pointed out that
> > this is something that was realised late on)
> > 2. MSIs are usually treated the same as DMA writes, in that they must be
> > mapped by the SMMU page tables so that they target a physical MSI
> > doorbell
> > 3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
> > doorbell built into the PCI RC)
> > 4. Platforms typically have some set of addresses that abort before
> > reaching the SMMU (e.g. because the PCI identifies them as P2P).
> >
> > All of this means that userspace (QEMU) needs to identify the memory
> > regions corresponding to points (3) and (4) and ensure that they are
> > not allocated in the guest physical (IPA) space. For platforms that can
> > remap the MSI doorbell as in (2), then some space also needs to be
> > allocated for that.
> >
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.
>
> Is my understanding correct, that you need to tell userspace about the
> location of the doorbell (in the IOVA space) in case (2), because even
> though the configuration of the device is handled by the (host) kernel
> through trapping of the BARs, we have to avoid the VFIO user programming
> the device to create other DMA transactions to this particular address,
> since that will obviously conflict and either not produce the desired
> DMA transactions or result in unintended weird interrupts?
Correct, if the MSI doorbell IOVA range overlaps RAM in the VM, then
it's potentially a DMA target and we'll get bogus data on DMA read from
the device, and lose data and potentially trigger spurious interrupts on
DMA write from the device. Thanks,
Alex
^ permalink raw reply
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Benjamin Herrenschmidt @ 2016-11-08 23:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108120323.GC15297@leverpostej>
On Tue, 2016-11-08 at 12:03 +0000, Mark Rutland wrote:
> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> >
> > For arm64, there is no I/O space as other architectural platforms, such as
> > X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> > such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> > known port addresses are used to control the corresponding target devices, for
> > example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> > normal MMIO mode in using.
>
> This has nothing to do with arm64. Hardware with this kind of indirect
> bus access could be integrated with a variety of CPU architectures. It
> simply hasn't been, yet.
On some ppc's we also use similar indirect access methods for IOs. We
have a generic infrastructure for re-routing some memory or IO regions
to hooks.
On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
> > To drive these devices, this patch introduces a method named indirect-IO.
> > In this method the in/out pair in arch/arm64/include/asm/io.h will be
> > redefined. When upper layer drivers call in/out with those known legacy port
> > addresses to access the peripherals, the hooking functions corrresponding to
> > those target peripherals will be called. Through this way, those upper layer
> > drivers which depend on in/out can run on Hip06 without any changes.
>
> As above, this has nothing to do with arm64, and as such, should live in
> generic code, exactly as we would do if we had higher-level ISA
> accessor ops.
>
> Regardless, given the multi-instance case, I don't think this is
> sufficient in general (and I think we need higher-level ISA accessors
> to handle the indirection).
Multi-instance with IO is tricky to do generically because archs already
have all sort of hacks to deal with the fact that inb/outb don't require
an explicit ioremap, so an IO resource can take all sort of shape depending
on the arch.
Overall it boils down to applying some kind of per-instance "offset" to
the IO port number though.
> [...]
>
> >
> > diff --git a/arch/arm64/include/asm/extio.h b/arch/arm64/include/asm/extio.h
> > new file mode 100644
> > index 0000000..6ae0787
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/extio.h
>
> >
> > +#ifndef __LINUX_EXTIO_H
> > +#define __LINUX_EXTIO_H
>
> This doesn't match the file naming, __ASM_EXTIO_H would be consistent
> with other arm64 headers.
>
> >
> > +
> > +struct extio_ops {
> > > > + unsigned long start;/* inclusive, sys io addr */
> > > > + unsigned long end;/* inclusive, sys io addr */
>
> Please put whitespace before inline comments.
>
> [...]
>
> >
> > > > +type in##bw(unsigned long addr) \
> > > > +{ \
> > > > > > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > > > > > + arm64_extio_ops->end < addr) \
> > > > > > + return read##bw(PCI_IOBASE + addr); \
> > > > > > + return arm64_extio_ops->pfin ? \
> > > > > > + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
> > > > > > + addr, sizeof(type)) : -1; \
> > > > +} \
> > > > + \
> > > > +void out##bw(type value, unsigned long addr) \
> > > > +{ \
> > > > > > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > > > > > + arm64_extio_ops->end < addr) \
> > > > > > + write##bw(value, PCI_IOBASE + addr); \
> > > > > > + else \
> > > > > > + if (arm64_extio_ops->pfout) \
> > > > + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > > > > > + addr, value, sizeof(type)); \
> > > > +} \
> > > > + \
> > > > +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
> > > > +{ \
> > > > > > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > > > > > + arm64_extio_ops->end < addr) \
> > > > > > + reads##bw(PCI_IOBASE + addr, buffer, count); \
> > > > > > + else \
> > > > > > + if (arm64_extio_ops->pfins) \
> > > > + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
> > > > > > + addr, buffer, sizeof(type), count); \
> > > > +} \
> > > > + \
> > > > +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
> > > > +{ \
> > > > > > + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> > > > > > + arm64_extio_ops->end < addr) \
> > > > > > + writes##bw(PCI_IOBASE + addr, buffer, count); \
> > > > > > + else \
> > > > > > + if (arm64_extio_ops->pfouts) \
> > > > + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
> > > > > > + addr, buffer, sizeof(type), count); \
> > +}
> > +
>
> So all PCI I/O will be slowed down by irrelevant checks when this is
> enabled?
>
> [...]
>
> >
> > +static inline void arm64_set_extops(struct extio_ops *ops)
> > +{
> > > > + if (ops)
> > > > + WRITE_ONCE(arm64_extio_ops, ops);
> > +}
>
> Why WRITE_ONCE()?
>
> Is this not protected/propagated by some synchronisation mechanism?
>
> WRITE_ONCE() is not sufficient to ensure that this is consistently
> observed by readers, and regardless, I don't see READ_ONCE() anywhere in
> this patch.
>
> This looks very suspicious.
>
> Thanks,
> Mark.
^ permalink raw reply
* [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Benjamin Herrenschmidt @ 2016-11-08 23:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108114953.GB15297@leverpostej>
On Tue, 2016-11-08 at 11:49 +0000, Mark Rutland wrote:
>
> My understanding of ISA (which may be flawed) is that it's not part of
> the PCI host bridge, but rather on x86 it happens to share the IO space
> with PCI.
Sort-of. On some systems it actually goes through PCI and there's a
PCI->ISA bridge that uses substractive decoding to the legacy devices.
> So, how about this becomes:
>
> ? Hisilicon Hip06 SoCs implement a Low Pin Count (LPC) controller, which
> ? provides access to some legacy ISA devices.
>
> I believe that we could theoretically have multiple independent LPC/ISA
> busses, as is possible with PCI on !x86 systems. If the current ISA code
> assumes a singleton bus, I think that's something that needs to be fixed
> up more generically.
>
> I don't see why we should need any architecture-specific code here. Why
> can we not fix up the ISA bus code in drivers/of/address.c such that it
> handles multiple ISA bus instances, and translates all sub-device
> addresses relative to the specific bus instance?
What in that code prevents that today ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH] ARM: socfpga_defconfig: enable FS configs to support Angstrom filesystem
From: Arnd Bergmann @ 2016-11-08 22:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108224750.30242-1-dinguyen@kernel.org>
On Tuesday, November 8, 2016 4:47:50 PM CET Dinh Nguyen wrote:
> Enables:
>
> CONFIG_AUTOFS4_FS=y
> CONFIG_NFSD=y
> CONFIG_NFS_V3_ACL=y
> CONFIG_NFSD_V3_ACL=y
> CONFIG_NFSD_V4=y
>
> Signed-off-by: Matthew Gerlach <mgerlach@opensource.altera.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
>
No objections to the patch, but the changelog should not state what
the patch clearly shows already but instead it should explain why
you do this.
The subject line mentions "to support Angstrom filesystem", but
it's not clear (at least not to me) what that filesystem is.
Arnd
^ permalink raw reply
* [PATCH] ARM: socfpga_defconfig: enable FS configs to support Angstrom filesystem
From: Dinh Nguyen @ 2016-11-08 22:47 UTC (permalink / raw)
To: linux-arm-kernel
Enables:
CONFIG_AUTOFS4_FS=y
CONFIG_NFSD=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFSD_V3_ACL=y
CONFIG_NFSD_V4=y
Signed-off-by: Matthew Gerlach <mgerlach@opensource.altera.com>
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
arch/arm/configs/socfpga_defconfig | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 18d3ec1..2e1d254 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -116,13 +116,18 @@ CONFIG_EXT2_FS=y
CONFIG_EXT2_FS_XATTR=y
CONFIG_EXT2_FS_POSIX_ACL=y
CONFIG_EXT3_FS=y
+CONFIG_AUTOFS4_FS=y
CONFIG_VFAT_FS=y
CONFIG_NTFS_FS=y
CONFIG_NTFS_RW=y
CONFIG_TMPFS=y
CONFIG_CONFIGFS_FS=y
CONFIG_NFS_FS=y
+CONFIG_NFS_V3_ACL=y
CONFIG_ROOT_NFS=y
+CONFIG_NFSD=y
+CONFIG_NFSD_V3_ACL=y
+CONFIG_NFSD_V4=y
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_ISO8859_1=y
CONFIG_PRINTK_TIME=y
--
2.8.3
^ permalink raw reply related
* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-08 22:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108164948.GG20591@arm.com>
On Tuesday, November 8, 2016 4:49:49 PM CET Will Deacon wrote:
> On Tue, Nov 08, 2016 at 04:33:44PM +0000, John Garry wrote:
> > On 08/11/2016 16:12, Will Deacon wrote:
> > >On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> > >Is there no way to make this slightly more generic, so that it can be
> > >re-used elsewhere? For example, if struct extio_ops was common, then
> > >you could have the singleton (which maybe should be an interval tree?),
> > >type definition, setter function and the BUILD_EXTIO invocations
> > >somewhere generic, rather than squirelled away in the arch backend.
> > >
> > The concern would be that some architecture which uses generic higher-level
> > ISA accessor ops, but have IO space, could be affected.
>
> You're already adding a Kconfig symbol for this stuff, so you can keep
> that if you don't want it on other architectures. I'm just arguing that
> plumbing drivers directly into arch code via arm64_set_extops is not
> something I'm particularly fond of, especially when it looks like it
> could be avoided with a small amount of effort.
Agreed, I initially suggested putting this into arch/arm64/, but there isn't
really a reason why it couldn't just live in lib/ with the header file
bits moved to include/asm-generic/io.h which we already use.
Arnd
^ permalink raw reply
* [GIT PULL] pxa-dt for v4.10
From: Robert Jarzmik @ 2016-11-08 22:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd, Kevin, and Olof,
This is the pxa pull request for 4.10 device-tree, can you please consider
pulling ?
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/rjarzmik/linux.git tags/pxa-dt-4.10
for you to fetch changes up to f409d2f134d499354dca0613693f27d8efd75c74:
ARM: dts: pxa: add pxa27x cpu operating points (2016-11-02 22:52:45 +0100)
----------------------------------------------------------------
This device-tree pxa update brings :
- pxa25x support
- cpu operating points in preparation for cpufreq-dt
- small fixes
----------------------------------------------------------------
Robert Jarzmik (4):
ARM: dts: add pxa25x .dtsi file
ARM: dts: pxa: fix gpio0 and gpio1 interrupts
ARM: dts: pxa: add pxa25x cpu operating points
ARM: dts: pxa: add pxa27x cpu operating points
Vijay Kumar (1):
Fix no. of gpio cells in the pxa gpio binding doucmentation
.../devicetree/bindings/gpio/mrvl-gpio.txt | 6 +-
arch/arm/boot/dts/pxa25x.dtsi | 117 +++++++++++++++++++++
arch/arm/boot/dts/pxa27x.dtsi | 40 +++++++
arch/arm/boot/dts/pxa2xx.dtsi | 4 +-
4 files changed, 163 insertions(+), 4 deletions(-)
create mode 100644 arch/arm/boot/dts/pxa25x.dtsi
--
Robert
^ permalink raw reply
* [GIT PULL] pxa for v4.10
From: Robert Jarzmik @ 2016-11-08 22:19 UTC (permalink / raw)
To: linux-arm-kernel
Hello Arnd, Kevin, Olof,
Please consider this pull request for pxa 4.10 cycle. This pull request strides
off from my usual ones as its spans more than just mach-pxa.
The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
are available in the git repository at:
https://github.com/rjarzmik/linux.git tags/pxa-for-4.10
for you to fetch changes up to e413bd33ac44b6d0bebc0ef2ac19cbe7558a7303:
ARM: pxa: fix pxa25x interrupt init (2016-11-05 21:48:18 +0100)
----------------------------------------------------------------
This is the pxa changes for v4.10 cycle.
This cycle is covering :
- some clock fixes common with sa1100 architecture
- the consequence of the pxa_camera conversion to v4l2
- a small irq related fix for pxa25x device-tree only
----------------------------------------------------------------
Robert Jarzmik (8):
ARM: sa11x0/pxa: acquire timer rate from the clock rate
watchdog: sa11x0/pxa: get rid of get_clock_tick_rate
ARM: sa11x0/pxa: get rid of get_clock_tick_rate
ARM: pxa: pxa_cplds: honor probe deferral
ARM: pxa: mioa701: use the new pxa_camera platform_data
ARM: pxa: ezx: use the new pxa_camera platform_data
ARM: pxa: em-x270: use the new pxa_camera platform_data
ARM: pxa: fix pxa25x interrupt init
Russell King - ARM Linux (1):
clk: pxa25x: OSTIMER0 clocks from the main oscillator
Wei Yongjun (1):
ARM: pxa: remove duplicated include from spitz.c
arch/arm/mach-pxa/corgi.c | 1 -
arch/arm/mach-pxa/em-x270.c | 89 +++++---------
arch/arm/mach-pxa/ezx.c | 176 +++++++++++----------------
arch/arm/mach-pxa/generic.c | 18 +--
arch/arm/mach-pxa/include/mach/hardware.h | 2 -
arch/arm/mach-pxa/mioa701.c | 13 +-
arch/arm/mach-pxa/pxa25x.c | 2 +-
arch/arm/mach-pxa/pxa_cplds_irqs.c | 11 +-
arch/arm/mach-pxa/spitz.c | 1 -
arch/arm/mach-sa1100/generic.c | 2 +-
arch/arm/mach-sa1100/include/mach/hardware.h | 4 -
drivers/clk/pxa/clk-pxa25x.c | 2 +-
drivers/clocksource/pxa_timer.c | 11 +-
drivers/watchdog/sa1100_wdt.c | 24 +++-
include/clocksource/pxa.h | 3 +-
15 files changed, 142 insertions(+), 217 deletions(-)
--
Robert
^ permalink raw reply
* KASAN & the vmalloc area
From: Dmitry Vyukov @ 2016-11-08 22:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161108190302.GH15297@leverpostej>
On Tue, Nov 8, 2016 at 11:03 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I see a while back [1] there was a discussion of what to do about KASAN
> and vmapped stacks, but it doesn't look like that was solved, judging by
> the vmapped stacks pull [2] for v4.9.
>
> I wondered whether anyone had looked at that since?
>
> I have an additional reason to want to dynamically allocate the vmalloc
> area shadow: it turns out that KASAN currently interacts rather poorly
> with the arm64 ptdump code.
>
> When KASAN is selected, we allocate shadow for the whole vmalloc area,
> using common zero pte, pmd, pud tables. Walking over these in the ptdump
> code takes a *very* long time (I've seen up to 15 minutes with
> KASAN_OUTLINE enabled). For DEBUG_WX [3], this means boot hangs for that
> long, too.
>
> If I don't allocate vmalloc shadow (and remove the apparently pointlesss
> shadow of the shadow area), and only allocate shadow for the image,
> fixmap, vmemmap and so on, that delay gets cut to a few seconds, which
> is tolerable for a debug configuration...
>
> ... however, things blow up when the kernel touches vmalloc'd memory for
> the first time, as we don't install shadow for that dynamically.
I've seen the same iteration slowness problem on x86 with
CONFIG_DEBUG_RODATA which walks all pages. The is about 1 minute, but
it is enough to trigger rcu stall warning.
The zero pud and vmalloc-ed stacks looks like different problems.
To overcome the slowness we could map zero shadow for vmalloc area lazily.
However for vmalloc-ed stacks we need to map actual memory, because
stack instrumentation will read/write into the shadow. One downside
here is that vmalloc shadow can be as large as 1:1 (if we allocate 1
page in vmalloc area we need to allocate 1 page for shadow).
Re slowness: could we just skip the KASAN zero puds (the top level)
while walking? Can they be interesting for anybody? We can just
pretend that they are not there. Looks like a trivial solution for the
problem at hand.
^ permalink raw reply
* [PATCHv3] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 21:50 UTC (permalink / raw)
To: linux-arm-kernel
Add a more specific board compatible entry for all of the SOCFPGA
Cyclone 5 based boards.
Signed-off-by: Dinh Nguyen <dinguyen@kernel.org>
---
v3: Be a bit more specific with the c5 dk and sockit, use
"altr,socfpga-cyclone5-socdk" and "terasic,socfpga-cyclone5-sockit"
v2: remove extra space and add a comma between compatible entries
---
arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_socdk.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_sockit.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_sodia.dts | 2 +-
arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
index afea364..5ecd2ef 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
@@ -18,7 +18,7 @@
/ {
model = "Terasic DE-0(Atlas)";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "terasic,de0-atlas", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
index 424523b..e5a98e5 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
@@ -19,7 +19,7 @@
/ {
model = "Aries/DENX MCV EVK";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "denx,mcvevk", "altr,socfpga-cyclone5", "altr,socfpga";
aliases {
ethernet0 = &gmac0;
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
index 15e43f4..7a5f42d 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
@@ -19,7 +19,7 @@
/ {
model = "Altera SOCFPGA Cyclone V SoC Development Kit";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "altr,socfpga-cyclone5-socdk", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
index 02e22f5..fcacaf7b 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
@@ -19,7 +19,7 @@
/ {
model = "Terasic SoCkit";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "terasic,socfpga-cyclone5-sockit", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
index 9aaf413..5b7e3c2 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
@@ -21,7 +21,7 @@
/ {
model = "Altera SOCFPGA Cyclone V SoC Macnica Sodia board";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "macnica,sodia", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "earlyprintk";
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
index b844473..363ee62 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
@@ -51,7 +51,7 @@
/ {
model = "samtec VIN|ING FPGA";
- compatible = "altr,socfpga-cyclone5", "altr,socfpga";
+ compatible = "samtec,vining", "altr,socfpga-cyclone5", "altr,socfpga";
chosen {
bootargs = "console=ttyS0,115200";
--
2.8.3
^ permalink raw reply related
* [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Robert Jarzmik @ 2016-11-08 21:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d57cdfba-da22-652d-3d75-6d1b754cf1d5@metafoo.de>
Lars-Peter Clausen <lars@metafoo.de> writes:
> On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
>> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
>> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
>
> In my opinion these should be inline functions rather than macros as that
> generates much more legible compiler errors e.g. in case there is a type
> mismatch.
Sure, why not.
>> +struct ac97_codec_driver {
>> + struct device_driver driver;
>> + int (*probe)(struct ac97_codec_device *);
>> + int (*remove)(struct ac97_codec_device *);
>> + int (*suspend)(struct ac97_codec_device *);
>> + int (*resume)(struct ac97_codec_device *);
>> + void (*shutdown)(struct ac97_codec_device *);
>
> The suspend, resume and shutdown callbacks are never used. Which is good,
> since all new frameworks should use dev_pm_ops. Just drop the from the struct.
Ok.
>> +/**
>> + * struct ac97_controller - The AC97 controller of the AC-Link
>> + * @ops: the AC97 operations.
>> + * @controllers: linked list of all existing controllers.
>> + * @dev: the device providing the AC97 controller.
>> + * @slots_available: the mask of accessible/scanable codecs.
>> + * @codecs: the 4 possible AC97 codecs (NULL if none found).
>> + * @codecs_pdata: platform_data for each codec (NULL if no pdata).
>> + *
>> + * This structure is internal to AC97 bus, and should not be used by the
>> + * controllers themselves, excepting for using @dev.
>> + */
>> +struct ac97_controller {
>> + const struct ac97_controller_ops *ops;
>> + struct list_head controllers;
>> + struct device *dev;
>
> I'd make the controller itself a struct dev, rather than just having the
> pointer to the parent. This is more idiomatic and matches what other
> subsystems do. It has several advantages, you get proper refcounting of your
> controller structure, the controller gets its own sysfs directory where the
> CODECs appear as children, you don't need the manual sysfs attribute
> creation and removal in ac97_controler_{un,}register().
If you mean having "struct device dev" instead of "struct device *dev", it has
also a drawback : all the legacy platforms have already a probing method, be
that platform data, device-tree or something else.
I'm a bit converned about the conversion toll. Maybe I've not understood your
point fully, so please feel free to explain, with an actual example even better.
>> + void (*wait)(struct ac97_controller *adrv, int slot);
>> + void (*init)(struct ac97_controller *adrv, int slot);
>
> Neither wait nor init are ever used.
This is because I've not begun to porting sound/pci/ac97_codec.c into
sound/ac97.
>> + if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
>> + return ERR_PTR(-ERANGE);
>
> I'd make this EINVAL.
Ok.
>> + adev = container_of(dev, struct ac97_codec_device, dev);
>
> to_ac97_device()
Sure.
>> + codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
>> + idx);
>> + codec->dev.init_name = codec_name;
>
> init_name is only for statically allocated devices. Use dev_set_name(dev,
> ...). No need for kasprintf() either as dev_set_name() takes a format string.
I'll try again, I seem to remember having tried that and it failed, but I can't
remember why.
> For this you need to split device_register into device_initialize() and
> device_add(). But usually that is what you want anyway.
Let me try again.
>> + ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
>> + "ac97_controller");
>
> Since the CODEC is a child of the controller this should not be necessary as
> this just points one directory up. It's like `ln -s .. parent`
This creates :
/sys/bus/ac97/devices/pxa2xx-ac97\:0/ac97_controller
Of course as you pointed out, it's a 'ln -s .. parent' like link, but it seems
to me very unfriendly to have :
- /sys/bus/ac97/devices/pxa2xx-ac97\:0/../ac97_operations
- while /sys/bus/ac97/devices/ac97_operations doesn't exist (obviously)
Mmm, I don't know for this one, my mind is not set, it's a bit hard to tell if
it's only an unecessary link bringing "comfort", or something usefull.
>
>> + if (ret)
>> + goto err_unregister_device;
>> +
>> + return 0;
>> +err_unregister_device:
>> + put_device(&codec->dev);
>> +err_free_codec:
>> + kfree(codec);
>
> Since the struct is reference counted, the freeing is done in the release
> callback and this leads to a double free.
A yes in the "goto err_unregister_device" case right, while it's necessary in
the "goto err_free_codec" case ... I need to rework that a bit.
>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>> +{
>> + int ret;
>> +
>> + drv->driver.bus = &ac97_bus_type;
>> +
>> + ret = driver_register(&drv->driver);
>> + if (!ret)
>> + ac97_rescan_all_controllers();
>
> Rescanning the bus when a new codec driver is registered should not be
> neccessary. The bus is scanned once when the controller is registered, this
> creates the device. The device driver core will take care of binding the
> device to the driver, if the driver is registered after thed evice.
That's because you suppose the initial scanning found all the ac97 codecs.
But that's an incomplete vision as a codec might be powered off at that time and
not seen by the first scanning, while the new scanning will discover it.
>> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
>> + if (ac97_ctrl->codecs[i])
>> + put_device(&ac97_ctrl->codecs[i]->dev);
>
> This should be device_unregister() to match the device_register() in
> ac97_codec_add().
Right.
>> +
>> + return 0;
>> +}
>> +
>> +static ssize_t cold_reset_store(struct device *dev,
>> + struct device_attribute *attr, const char *buf,
>> + size_t len)
>> +{
>> + struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
>> +
>> + if (!dev)
>> + return -ENODEV;
>
> dev is never NULL here.
Ok.
> And for the ac97_ctrl there is a race condition. It could be unregistered and
> freed after ac97_ctrl_find() returned sucessfully, but before ac97_ctrl->ops
> is used.
A good catch, the ac97_controllers_mutex is missing.
> Same here.
Indeed.
>> +
>> + ac97_ctrl->ops->warm_reset(ac97_ctrl);
>> + return len;
>> +}
>> +static DEVICE_ATTR_WO(warm_reset);
>> +
>> +static struct attribute *ac97_controller_device_attrs[] = {
>> + &dev_attr_cold_reset.attr,
>> + &dev_attr_warm_reset.attr,
>> + NULL
>> +};
>
> This adds new userspace ABI that is not documented at the moment.
Very true. And as all userspace interface, it needs to be discussed. If nobody
complains, I'll add the documentation for next patch round.
>
>> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
>> + struct device *dev,
>> + unsigned short slots_available,
>> + void **codecs_pdata)
>
> In my opinion this should return a handle to a ac97 controller which can
> then be passed to snd_ac97_controller_unregister(). This is in my opinion
> the better approach rather than looking up the controller by parent device.
This is another "legacy drivers" issue.
The legacy driver (the one probed by platform_data or devicetree) doesn't
necessarily have a private structure to store this ac97_controller pointer. This
enables an "easier" porting of existing drivers.
>> +/**
>> + * snd_ac97_controller_unregister - unregister an ac97 controller
>> + * @dev: the device previously provided to ac97_controller_register()
>> + *
>> + * Returns 0 on success, negative upon error
>
> Unregister must not be able to fail. Hotunplug is one of the core concepts
> of the device driver model and there is really nothing that can be done to
> prevent a device from disappearing, so there is no sensible way of handling
> the error (and your pxa driver modifications simply ignore it as well).
>
> This also means the framework needs to cope with the case where the
> controller is removed and the CODEC devices are still present. All future
> operations should return -ENODEV in that case.
Ah ... that will require a serious modification, and I see your point, so I'll
prepare this for next patchset.
>> +static struct bus_type ac97_bus_type = {
>> + .name = "ac97",
>> + .dev_attrs = ac97_dev_attrs,
>
> dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
Ok.
>> index 000000000000..a835f03744bf
>> --- /dev/null
>> +++ b/sound/ac97/codec.c
>> @@ -0,0 +1,15 @@
>> +/*
>> + * Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
>> + *
>> + * 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
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <sound/ac97_codec.h>
>> +#include <sound/ac97/codec.h>
>> +#include <sound/ac97/controller.h>
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <sound/soc.h> /* For compat_ac97_* */
>> +
>
> I'm not sure I understand what this file does.
Ah yes, I'll remove it.
It's the future conversion of sound/pci/ac97_codec.c, but it's ... empty so far.
Thanks very much for the very detailed review.
--
Robert
^ permalink raw reply
* [PATCHv2] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 20:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161101222352.GA6328@localhost>
On 11/01/2016 05:23 PM, Olof Johansson wrote:
> On Tue, Nov 01, 2016 at 03:56:52PM -0500, Dinh Nguyen wrote:
>> Add a more specific board compatible entry for all of the SOCFPGA
>> Cyclone 5 based boards.
>>
[snip]
>> / {
>> model = "Altera SOCFPGA Cyclone V SoC Development Kit";
>> - compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> + compatible = "altr,socdk", "altr,socfpga-cyclone5", "altr,socfpga";
>
> This looks a little too generic, what if there's another dk with another
> SoC down the road?
>
Right...I'll change it to "altr,socfpga-cyclone5-socdk",
>> chosen {
>> bootargs = "earlyprintk";
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> index 02e22f5..c5623a7 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> @@ -19,7 +19,7 @@
>>
>> / {
>> model = "Terasic SoCkit";
>> - compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> + compatible = "terasic,sockit", "altr,socfpga-cyclone5", "altr,socfpga";
>
> Same thing here, this seems a bit on the generic side.
>
perhaps "terasic,socfpga-cyclone5-sockit" ?
Thanks for reviewing.
Dinh
^ 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