* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-11 23:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411140857.GA6821@red-moon>
On Tue, 2017-04-11 at 15:08 +0100, Lorenzo Pieralisi wrote:
> On Tue, Apr 11, 2017 at 11:38:26PM +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > > This patch series[1] is a v3 of a previous version:
> > >
> > > v2: https://lkml.org/lkml/2017/3/27/220
> >
> > I am not a fan of this at All.
> >
> > That whole concept of "ioremap_nopost" is simply not applicable to the
> > majority of architectures and certainly not in a way that can apply to
> > arbitrary mappings.
> >
> > It's also very wrong to provide a "default" operation whose semantics
> > are weaker than what it's supposed to implement. Very wrong actually.
> > People will use it assuming the non-posted behaviour and things will
> > break in subtle way when it cannot be provided.
>
> Well, what's very wrong for you it is not very wrong for others
> (it is just v3, that's fine, see thread below).
Maybe, but I don't see in what universe it is ok to have something
defined for the stronger ordering semantics it provide silently
fallback to weaker semantics.
> I can easily make ioremap_nopost() mirror ioremap_uc() (ie return
> NULL unless overriden so that basically you can't use in on an arch
> that can't provide its semantics) but then that becomes very wrong
> for other reviewers.
Those reviewers are WRONG :-)
> https://lkml.org/lkml/2017/4/6/396
>
> > What exactly are you trying to fix here ?
>
> I wrote in the commit logs and cover letter what I am fixing here.
Right right, what *actual bug you have observed* are you trying to fix
?
I'm pretty such close to all non-x86 archs had that "problem" since the
dawn of time and it has never hurt anybody.
That said, I don't think it makes sense to "solve" it by creating a
"generic" mapping semantic that is basically impossible to implement on
most architectures out there (and cannot be emulated).
This is a problem to be solved by the bridge itself. If ARM has a
mapping attribute to make stores non-posted, keep this an ARM specific
attribute at this stage I'd say.
> Anyway:
>
> "The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> Posting") mandate non-posted configuration transactions. As further
> highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering
> Considerations for the Enhanced Configuration Access Mechanism"),
> through ECAM and ECAM-derivative configuration mechanism, the memory
> mapped transactions from the host CPU into Configuration Requests on the
> PCI express fabric may create ordering problems for software because
> writes to memory address are typically posted transactions (unless the
> architecture can enforce through virtual address mapping non-posted
> write transactions behaviour) but writes to Configuration Space are not
> posted on the PCI express fabric."
>
> On ARM64:
>
> "This rule is reinforced by the ARM v8 architecture reference manual
> (issue A.k, Early Write Acknowledgment) that explicitly recommends
> that No Early Write Acknowledgment attribute should be used to map
> PCI configuration (write) transactions."
>
> > If a given PCIe host bridge (architecture specific) require a special
> > sauce to provide the illusion of non-posting, then implement this in
> > the actual root complex code.
> >
> > BTW. I'm pretty sure we "accidentally" made config writes posted at
> > least to the PHB on a number of powerpc systems forever and we *never*
> > had a problem because of it ;)
>
> Ok so we should ignore the PCIe specifications and ARM v8 reference
> manual waiting for a kernel bug to appear ? Is that what you suggest
> doing ?
>
> Lorenzo
>
> > > v2 -> v3:
> > > - Created a default ioremap_nopost() implementation in a
> > > separate
> > > ??asm-generic header and patched all arches to make use of it
> > > - Removed PCI drivers patches from the series to simplify the
> > > ??review, they will be posted separately once the
> > > ioremap_nopost()
> > > ??interface is settled
> > > - Fixed devm_ioremap_* BUS offset comments and implemented
> > > ??nopost interface on top of it
> > > - Added collected tags
> > >
> > > v1: https://lkml.org/lkml/2017/2/27/228
> > >
> > > v1 -> v2:
> > > - Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> > > ??interface
> > > - Added pgprot_nonposted
> > > - Fixed build errors on arches not relying on asm-generic
> > > headers
> > > - Added PCI versatile host controller driver patch
> > > - Added missing config space remapping to hisilicon host
> > > controller
> > >
> > > ---------------------
> > > Original cover letter
> > > ---------------------
> > >
> > > PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> > > and Posting") strictly require PCI configuration and I/O Address
> > > space
> > > write transactions to be non-posted.
> > >
> > > Current crop of DT/ACPI PCI host controllers drivers relies on
> > > the ioremap interface to map ECAM and ECAM-derivative PCI config
> > > regions and pci_remap_iospace() to create a VMA for mapping
> > > PCI host bridge I/O Address space transactions to CPU virtual address
> > > space.
> > >
> > > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> > > configuration non-posted write transactions requirement, because it
> > > provides a memory mapping that issues "bufferable" or, in PCI terms
> > > "posted" write transactions. Likewise, the current
> > > pci_remap_iospace()
> > > implementation maps the physical address range that the PCI
> > > translates
> > > to I/O space cycles to virtual address space through pgprot_device()
> > > attributes that on eg ARM64 provides a memory mapping issuing
> > > posted writes transactions, which is not PCI specifications
> > > compliant.
> > >
> > > This patch series[1] addresses both issues in one go:
> > >
> > > - It updates the pci_remap_iospace() function to use a page mapping
> > > ? that guarantees non-posted write transactions for I/O space
> > > addresses
> > > - It adds a kernel API to remap PCI config space resources, so that
> > > ? architecture can override it with a mapping implementation that
> > > ? guarantees PCI specifications compliancy wrt non-posted write
> > > ? configuration transactions
> > > - It updates all PCI host controller implementations (and the generic
> > > ? ECAM layer) to use the newly introduced mapping interface
> > >
> > > Tested on Juno ECAM based interface (DT/ACPI).
> > >
> > > Non-ECAM PCI host controller drivers patches need checking to make
> > > sure that:
> > >
> > > - I patched the correct resource region mapping for config space
> > > - There are not any other ways to ensure posted-write completion
> > > ? in the respective pci_ops that make the relevant patch unnecessary
> > >
> > > [1]
> > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> > > pci/config-io-mappings-fix-v3
> > >
> > > Lorenzo Pieralisi (32):
> > > ? PCI: remove __weak tag from pci_remap_iospace()
> > > ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> > > ? PCI: fix pci_remap_iospace() remap attribute
> > > ? asm-generic: add ioremap_nopost() remap interface
> > > ? alpha: include default ioremap_nopost() implementation
> > > ? avr32: include default ioremap_nopost() implementation
> > > ? arc: include default ioremap_nopost() implementation
> > > ? cris: include default ioremap_nopost() implementation
> > > ? frv: include default ioremap_nopost() implementation
> > > ? hexagon: include default ioremap_nopost() implementation
> > > ? ia64: include default ioremap_nopost() implementation
> > > ? m32r: include default ioremap_nopost() implementation
> > > ? m68k: include default ioremap_nopost() implementation
> > > ? metag: include default ioremap_nopost() implementation
> > > ? microblaze: include default ioremap_nopost() implementation
> > > ? mips: include default ioremap_nopost() implementation
> > > ? mn10300: include default ioremap_nopost() implementation
> > > ? nios2: include default ioremap_nopost() implementation
> > > ? openrisc: include default ioremap_nopost() implementation
> > > ? parisc: include default ioremap_nopost() implementation
> > > ? powerpc: include default ioremap_nopost() implementation
> > > ? s390: include default ioremap_nopost() implementation
> > > ? sh: include default ioremap_nopost() implementation
> > > ? sparc: include default ioremap_nopost() implementation
> > > ? tile: include default ioremap_nopost() implementation
> > > ? unicore32: include default ioremap_nopost() implementation
> > > ? x86: include default ioremap_nopost() implementation
> > > ? xtensa: include default ioremap_nopost() implementation
> > > ? arm64: implement ioremap_nopost() interface
> > > ? arm: implement ioremap_nopost() interface
> > > ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> > > description
> > > ? lib: implement Devres ioremap_nopost() interface
> > >
> > > ?Documentation/driver-model/devres.txt |??3 ++
> > > ?arch/alpha/include/asm/io.h???????????|??1 +
> > > ?arch/arc/include/asm/io.h?????????????|??1 +
> > > ?arch/arm/include/asm/io.h?????????????|??9 ++++
> > > ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> > > ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> > > ?arch/arm64/include/asm/io.h???????????| 12 +++++
> > > ?arch/avr32/include/asm/io.h???????????|??1 +
> > > ?arch/cris/include/asm/io.h????????????|??1 +
> > > ?arch/frv/include/asm/io.h?????????????|??1 +
> > > ?arch/hexagon/include/asm/io.h?????????|??2 +
> > > ?arch/ia64/include/asm/io.h????????????|??1 +
> > > ?arch/m32r/include/asm/io.h????????????|??1 +
> > > ?arch/m68k/include/asm/io.h????????????|??1 +
> > > ?arch/metag/include/asm/io.h???????????|??2 +
> > > ?arch/microblaze/include/asm/io.h??????|??1 +
> > > ?arch/mips/include/asm/io.h????????????|??1 +
> > > ?arch/mn10300/include/asm/io.h?????????|??1 +
> > > ?arch/nios2/include/asm/io.h???????????|??1 +
> > > ?arch/openrisc/include/asm/io.h????????|??2 +
> > > ?arch/parisc/include/asm/io.h??????????|??1 +
> > > ?arch/powerpc/include/asm/io.h?????????|??1 +
> > > ?arch/s390/include/asm/io.h????????????|??1 +
> > > ?arch/sh/include/asm/io.h??????????????|??1 +
> > > ?arch/sparc/include/asm/io.h???????????|??1 +
> > > ?arch/tile/include/asm/io.h????????????|??1 +
> > > ?arch/unicore32/include/asm/io.h???????|??1 +
> > > ?arch/x86/include/asm/io.h?????????????|??1 +
> > > ?arch/xtensa/include/asm/io.h??????????|??1 +
> > > ?drivers/pci/pci.c?????????????????????|??4 +-
> > > ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> > > ?include/asm-generic/pgtable.h?????????|??4 ++
> > > ?include/linux/device.h????????????????|??2 +
> > > ?include/linux/io.h????????????????????|??2 +
> > > ?lib/devres.c??????????????????????????| 84
> > > +++++++++++++++++++++++++++++++++--
> > > ?35 files changed, 167 insertions(+), 5 deletions(-)
> > > ?create mode 100644 include/asm-generic/ioremap-nopost.h
> > >
^ permalink raw reply
* [PATCH V8 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT
From: Bjorn Helgaas @ 2017-04-11 22:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7b94bfbb-b3e6-cbc3-06a1-4490f4142ffe@codeaurora.org>
On Tue, Apr 11, 2017 at 05:27:59PM -0400, Sinan Kaya wrote:
> On 4/11/2017 5:19 PM, Bjorn Helgaas wrote:
> > Hi Sinan,
> >
> > What are these patches based on? I normally apply things to my "master"
> > branch, which is typically -rc1 or -rc2. Right now it's v4.11-rc1, and
> > these don't apply cleanly:
> >
> > 04:17:59 ~/linux (master)$ stg branch --create pci/aspm-sk-v8 v4.11-rc1
> > Checking for changes in the working directory ... done
> > Don't know how to determine parent branch from "v4.11-rc1"
> > Branch "pci/aspm-sk-v8" created
> > 04:18:18 ~/linux (pci/aspm-sk-v8)$ stg import -M --sign m/sk-v8
> > Checking for changes in the working directory ... done
> > Importing patch "pci-aspm-introduce" ... done
> > Importing patch "pci-aspm-split-pci_aspm_init" ... done
> > Importing patch "pci-aspm-add-init-hook-to" ... error: patch failed: drivers/pci/pcie/aspm.c:798
> > error: drivers/pci/pcie/aspm.c: patch does not apply
> > stg import: Diff does not apply cleanly
>
> This is what I have.
>
> 41a7175 PCI/ASPM: move link_state cleanup to bridge remove
> 3b646e2 PCI/ASPM: save power on values during bridge init
> 181510d PCI/ASPM: add init hook to device_add
> 282d008 PCI/ASPM: split pci_aspm_init() into two
> 8e89fc2 PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities()
> 97da385 Linux 4.11-rc3
Thanks, I guess I should have figured that out since my for-linus branch
includes an ASPM patch.
^ permalink raw reply
* linux-next: manual merge of the mvebu tree with the arm-soc tree
From: Stephen Rothwell @ 2017-04-11 22:34 UTC (permalink / raw)
To: linux-arm-kernel
Hi all,
Today's linux-next merge of the mvebu tree got a conflict in:
arch/arm64/configs/defconfig
between commit:
3c9d36192802 ("arm64: set CONFIG_MMC_BCM2835=y in defconfig")
from the arm-soc tree and commit:
6ff829553345 ("arm64: configs: enable SDHCI driver for Xenon")
from the mvebu tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
BTW, that arm-soc commit has no Signed-off-by from its committer :-(
--
Cheers,
Stephen Rothwell
diff --cc arch/arm64/configs/defconfig
index ab4461b6b226,93b0aab959c0..000000000000
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@@ -402,7 -401,7 +402,8 @@@ CONFIG_MMC_DW_EXYNOS=
CONFIG_MMC_DW_K3=y
CONFIG_MMC_DW_ROCKCHIP=y
CONFIG_MMC_SUNXI=y
+CONFIG_MMC_BCM2835=y
+ CONFIG_MMC_SDHCI_XENON=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
^ permalink raw reply
* [PATCH v2] staging: media/platform/bcm2835: remove gstreamer workaround
From: Kevin Wern @ 2017-04-11 22:17 UTC (permalink / raw)
To: linux-arm-kernel
Gstreamer's v4l2src reacted poorly to certain outputs from the bcm2835
video driver's ioctl ops function vidioc_enum_framesizes, so a
workaround was created that could be activated by user input. This
workaround would replace the driver's ioctl ops struct with another,
similar struct--only with no function pointed to by
vidioc_enum_framesizes. With no response, gstreamer would attempt to
continue with some default settings that happened to work better.
However, this bug has been fixed in gstreamer since 2014, so we
shouldn't include this workaround in the stable version of the driver.
Signed-off-by: Kevin Wern <kevin.m.wern@gmail.com>
---
V2: patch against staging-next instead of torvalds/master
drivers/staging/vc04_services/bcm2835-camera/TODO | 5 --
.../vc04_services/bcm2835-camera/bcm2835-camera.c | 60 ----------------------
2 files changed, 65 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/TODO b/drivers/staging/vc04_services/bcm2835-camera/TODO
index 61a5099..0ab9e88 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/TODO
+++ b/drivers/staging/vc04_services/bcm2835-camera/TODO
@@ -32,8 +32,3 @@ We should have VCHI create a platform device once it's initialized,
and have this driver bind to it, so that we automatically load the
v4l2 module after VCHI loads.
-5) Drop the gstreamer workaround.
-
-This was a temporary workaround for a bug that was fixed mid-2014, and
-we should remove it before stabilizing the driver.
-
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 86bbd6e..a11e047 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -66,19 +66,6 @@ MODULE_PARM_DESC(max_video_width, "Threshold for video mode");
module_param(max_video_height, int, 0644);
MODULE_PARM_DESC(max_video_height, "Threshold for video mode");
-/* Gstreamer bug https://bugzilla.gnome.org/show_bug.cgi?id=726521
- * v4l2src does bad (and actually wrong) things when the vidioc_enum_framesizes
- * function says type V4L2_FRMSIZE_TYPE_STEPWISE, which we do by default.
- * It's happier if we just don't say anything at all, when it then
- * sets up a load of defaults that it thinks might work.
- * If gst_v4l2src_is_broken is non-zero, then we remove the function from
- * our function table list (actually switch to an alternate set, but same
- * result).
- */
-static int gst_v4l2src_is_broken;
-module_param(gst_v4l2src_is_broken, int, 0644);
-MODULE_PARM_DESC(gst_v4l2src_is_broken, "If non-zero, enable workaround for Gstreamer");
-
/* global device data array */
static struct bm2835_mmal_dev *gdev[MAX_BCM2835_CAMERAS];
@@ -1454,48 +1441,6 @@ static const struct v4l2_ioctl_ops camera0_ioctl_ops = {
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
};
-static const struct v4l2_ioctl_ops camera0_ioctl_ops_gstreamer = {
- /* overlay */
- .vidioc_enum_fmt_vid_overlay = vidioc_enum_fmt_vid_overlay,
- .vidioc_g_fmt_vid_overlay = vidioc_g_fmt_vid_overlay,
- .vidioc_try_fmt_vid_overlay = vidioc_try_fmt_vid_overlay,
- .vidioc_s_fmt_vid_overlay = vidioc_s_fmt_vid_overlay,
- .vidioc_overlay = vidioc_overlay,
- .vidioc_g_fbuf = vidioc_g_fbuf,
-
- /* inputs */
- .vidioc_enum_input = vidioc_enum_input,
- .vidioc_g_input = vidioc_g_input,
- .vidioc_s_input = vidioc_s_input,
-
- /* capture */
- .vidioc_querycap = vidioc_querycap,
- .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
- .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
- .vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
- .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
-
- /* buffer management */
- .vidioc_reqbufs = vb2_ioctl_reqbufs,
- .vidioc_create_bufs = vb2_ioctl_create_bufs,
- .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
- .vidioc_querybuf = vb2_ioctl_querybuf,
- .vidioc_qbuf = vb2_ioctl_qbuf,
- .vidioc_dqbuf = vb2_ioctl_dqbuf,
- /* Remove this function ptr to fix gstreamer bug
- * .vidioc_enum_framesizes = vidioc_enum_framesizes,
- */
- .vidioc_enum_frameintervals = vidioc_enum_frameintervals,
- .vidioc_g_parm = vidioc_g_parm,
- .vidioc_s_parm = vidioc_s_parm,
- .vidioc_streamon = vb2_ioctl_streamon,
- .vidioc_streamoff = vb2_ioctl_streamoff,
-
- .vidioc_log_status = v4l2_ctrl_log_status,
- .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
- .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
-};
-
/* ------------------------------------------------------------------
* Driver init/finalise
* ------------------------------------------------------------------
@@ -1811,11 +1756,6 @@ static int __init bm2835_mmal_init_device(struct bm2835_mmal_dev *dev,
int ret;
*vfd = vdev_template;
- if (gst_v4l2src_is_broken) {
- v4l2_info(&dev->v4l2_dev,
- "Work-around for gstreamer issue is active.\n");
- vfd->ioctl_ops = &camera0_ioctl_ops_gstreamer;
- }
vfd->v4l2_dev = &dev->v4l2_dev;
--
2.7.4
^ permalink raw reply related
* [PATCH V8 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT
From: Sinan Kaya @ 2017-04-11 21:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411211924.GC14778@bhelgaas-glaptop.roam.corp.google.com>
On 4/11/2017 5:19 PM, Bjorn Helgaas wrote:
> Hi Sinan,
>
> What are these patches based on? I normally apply things to my "master"
> branch, which is typically -rc1 or -rc2. Right now it's v4.11-rc1, and
> these don't apply cleanly:
>
> 04:17:59 ~/linux (master)$ stg branch --create pci/aspm-sk-v8 v4.11-rc1
> Checking for changes in the working directory ... done
> Don't know how to determine parent branch from "v4.11-rc1"
> Branch "pci/aspm-sk-v8" created
> 04:18:18 ~/linux (pci/aspm-sk-v8)$ stg import -M --sign m/sk-v8
> Checking for changes in the working directory ... done
> Importing patch "pci-aspm-introduce" ... done
> Importing patch "pci-aspm-split-pci_aspm_init" ... done
> Importing patch "pci-aspm-add-init-hook-to" ... error: patch failed: drivers/pci/pcie/aspm.c:798
> error: drivers/pci/pcie/aspm.c: patch does not apply
> stg import: Diff does not apply cleanly
This is what I have.
41a7175 PCI/ASPM: move link_state cleanup to bridge remove
3b646e2 PCI/ASPM: save power on values during bridge init
181510d PCI/ASPM: add init hook to device_add
282d008 PCI/ASPM: split pci_aspm_init() into two
8e89fc2 PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities()
97da385 Linux 4.11-rc3
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH V2] clk: hi6220: Add the hi655x's pmic clock
From: Daniel Lezcano @ 2017-04-11 21:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411140613.jvhzsvrxa4evwxjh@dell>
On Tue, Apr 11, 2017 at 03:06:13PM +0100, Lee Jones wrote:
> On Sat, 08 Apr 2017, Daniel Lezcano wrote:
>
> > The hi655x multi function device is a PMIC providing regulators.
> >
> > The PMIC also provides a clock for the WiFi and the Bluetooth, let's implement
> > this clock in order to add it in the hi655x MFD and allow proper wireless
> > initialization.
> >
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> >
> > Changelog:
> >
> > V2:
> > - Added COMPILE_TEST option, compiled on x86
> > - Removed useless parenthesis
> > - Used of_clk_hw_simple_get() instead of deref dance
> > - Do bailout if the clock-names is not specified
> > - Rollback on error
> > - Folded mfd line change and binding
>
> Why did you do that?
I thought as the V1 had comments you would have waited for the V2 and as it was
trivial enough, it could be folded and picked up via the clk tree via with your
acked-by.
I realize it was not a good idea.
Do you want to drop it from your tree or shall I resubmit a V3 without the mfd
change?
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH V8 0/5] PCI/ASPM: reconfigure ASPM following hotplug for POLICY_DEFAULT
From: Bjorn Helgaas @ 2017-04-11 21:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491627351-1111-1-git-send-email-okaya@codeaurora.org>
On Sat, Apr 08, 2017 at 12:55:46AM -0400, Sinan Kaya wrote:
> When the operating system is booted with the default ASPM policy
> (POLICY_DEFAULT), current code is querying the enable/disable
> states from ASPM registers to determine the policy.
>
> For example, a BIOS could set the power saving state to performance
> and clear all ASPM control registers. A balanced ASPM policy could
> enable L0s and disable L1. A power conscious BIOS could enable both
> L0s and L1 to trade off latency and performance vs. power.
>
> After hotplug removal, pcie_aspm_exit_link_state() function clears
> the ASPM registers. An insertion following hotplug removal reads
> incorrect policy as ASPM disabled even though ASPM was enabled
> during boot.
>
> This is caused by the fact that same function is used for reconfiguring
> ASPM regardless of the power on state.
>
> ------------------------
> Changes from v7 (https://www.spinics.net/lists/arm-kernel/msg573034.html)
> ------------------------
> - enable LTSSM as a best effort if the endpoint is not connected during
> boot
>
>
> Sinan Kaya (5):
> PCI/ASPM: introduce pci_aspm_init() and add to pci_init_capabilities()
> PCI/ASPM: split pci_aspm_init() into two
> PCI/ASPM: add init hook to device_add
> PCI/ASPM: save power on values during bridge init
> PCI/ASPM: move link_state cleanup to bridge remove
>
> drivers/pci/pcie/aspm.c | 149 +++++++++++++++++++++++++++++++++---------------
> drivers/pci/probe.c | 3 +
> drivers/pci/remove.c | 3 +-
> include/linux/pci.h | 2 +
> 4 files changed, 110 insertions(+), 47 deletions(-)
Hi Sinan,
What are these patches based on? I normally apply things to my "master"
branch, which is typically -rc1 or -rc2. Right now it's v4.11-rc1, and
these don't apply cleanly:
04:17:59 ~/linux (master)$ stg branch --create pci/aspm-sk-v8 v4.11-rc1
Checking for changes in the working directory ... done
Don't know how to determine parent branch from "v4.11-rc1"
Branch "pci/aspm-sk-v8" created
04:18:18 ~/linux (pci/aspm-sk-v8)$ stg import -M --sign m/sk-v8
Checking for changes in the working directory ... done
Importing patch "pci-aspm-introduce" ... done
Importing patch "pci-aspm-split-pci_aspm_init" ... done
Importing patch "pci-aspm-add-init-hook-to" ... error: patch failed: drivers/pci/pcie/aspm.c:798
error: drivers/pci/pcie/aspm.c: patch does not apply
stg import: Diff does not apply cleanly
^ permalink raw reply
* [PATCH 1/2] dt-bindings: Document STM32 I2S bindings
From: Mark Brown @ 2017-04-11 21:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <73caede5-ea6f-8b2b-2270-227f7ef31823@st.com>
On Tue, Apr 11, 2017 at 03:44:52PM +0000, Olivier MOYSAN wrote:
> However, regarding STM32 I2S driver, I'm wondering if selecting DAI
> is the best way to configure interface as tx, rx or fd.
Why do you even need to configure this?
> Maybe, it is more relevant to configure DAI according to DMA
> configuration from I2S node.
> This would moreover avoid to allocate 2 dmas channels when not
> necessary (tx or rx only).
> If you agree with this, I will implement this change in a v2.
That sounds wrong, I'd expect this wiring to be done statically as part
of the .dtsi for the SoC (or just grabbed as needed at runtime if
things are flexbile enough) rather than being a configuration thing done
per board... I had thought that this was configuration reflecting
different ways of taping out the IP with different feature sets, is that
not the case?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/1dcedbfa/attachment.sig>
^ permalink raw reply
* [PATCH v2 3/3] ARM: dts: da850: Add node for LEGO MINDSTORMS EV3 Battery
From: David Lechner @ 2017-04-11 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491944723-32174-1-git-send-email-david@lechnology.com>
This adds a new node to the LEGO MINDSTORMS EV3 device tree for the battery.
Signed-off-by: David Lechner <david@lechnology.com>
---
arch/arm/boot/dts/da850-lego-ev3.dts | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lego-ev3.dts b/arch/arm/boot/dts/da850-lego-ev3.dts
index c20580a..99cd561 100644
--- a/arch/arm/boot/dts/da850-lego-ev3.dts
+++ b/arch/arm/boot/dts/da850-lego-ev3.dts
@@ -172,6 +172,18 @@
gpio = <&gpio 111 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
+
+ /*
+ * The EV3 can use 6-AA batteries or a rechargeable Li-ion battery pack.
+ */
+ battery {
+ pinctrl-names = "default";
+ pintctrl-0 = <&battery_pins>;
+ compatible = "lego,ev3-battery";
+ io-channels = <&adc 4>, <&adc 3>;
+ io-channel-names = "voltage", "current";
+ rechargeable-gpios = <&gpio 136 GPIO_ACTIVE_LOW>;
+ };
};
&pmx_core {
@@ -235,6 +247,15 @@
0x34 0x00000008 0x0000000f
>;
};
+
+ battery_pins: pinmux_battery_pins {
+ pinctrl-single,bits = <
+ /* GP0[6] */
+ 0x04 0x00000080 0x000000f0
+ /* GP8[8] */
+ 0x4c 0x00000080 0x000000f0
+ >;
+ };
};
&pinconf {
@@ -349,6 +370,13 @@
&gpio {
status = "okay";
+
+ /* Don't pull down battery voltage adc io channel */
+ batt_volt_en {
+ gpio-hog;
+ gpios = <6 GPIO_ACTIVE_HIGH>;
+ output-low;
+ };
};
&usb_phy {
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/3] power: supply: New driver for LEGO MINDSTORMS EV3 battery
From: David Lechner @ 2017-04-11 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491944723-32174-1-git-send-email-david@lechnology.com>
This adds a new driver for the LEGO MINDSTORMS EV3 battery. The EV3 is
an embedded ARM device that can use 6 AA batteries or a special rechargeable
Li-ion battery pack. The rechargeable battery pack presses a special key
switch in the battery compartment to indicate that it is present.
The EV3 is only capable of monitoring battery voltage and current. The
charging circuit is built into the rechargeable battery pack and there is
no way to communicate with is, so we can't provide any information about
charging status.
When not using the rechargeable battery pack, it is most common to use
alkaline batteries to power the device, but it is also common for people to
use rechargeable NiMH batteries. Since there is not a way to automatically
differentiate between these, the technology property is made writable.
Signed-off-by: David Lechner <david@lechnology.com>
---
drivers/power/supply/Kconfig | 6 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/lego_ev3_battery.c | 228 ++++++++++++++++++++++++++++++++
3 files changed, 235 insertions(+)
create mode 100644 drivers/power/supply/lego_ev3_battery.c
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index cc50e15..e00bf25 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -117,6 +117,12 @@ config BATTERY_DS2782
Say Y here to enable support for the DS2782/DS2786 standalone battery
gas-gauge.
+config BATTERY_LEGO_EV3
+ tristate "LEGO MINDSTORMS EV3 battery"
+ depends on OF && IIO && GPIOLIB
+ help
+ Say Y here to enable support for the LEGO MINDSTORMS EV3 battery.
+
config BATTERY_PMU
tristate "Apple PMU battery"
depends on PPC32 && ADB_PMU
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 3789a2c..dfeadbc 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
obj-$(CONFIG_BATTERY_DS2782) += ds2782_battery.o
obj-$(CONFIG_BATTERY_GAUGE_LTC2941) += ltc2941-battery-gauge.o
obj-$(CONFIG_BATTERY_GOLDFISH) += goldfish_battery.o
+obj-$(CONFIG_BATTERY_LEGO_EV3) += lego_ev3_battery.o
obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o
obj-$(CONFIG_BATTERY_OLPC) += olpc_battery.o
obj-$(CONFIG_BATTERY_TOSA) += tosa_battery.o
diff --git a/drivers/power/supply/lego_ev3_battery.c b/drivers/power/supply/lego_ev3_battery.c
new file mode 100644
index 0000000..7b993d6
--- /dev/null
+++ b/drivers/power/supply/lego_ev3_battery.c
@@ -0,0 +1,228 @@
+/*
+ * Battery driver for LEGO MINDSTORMS EV3
+ *
+ * Copyright (C) 2017 David Lechner <david@lechnology.com>
+ *
+ * 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.
+
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+
+struct lego_ev3_battery {
+ struct iio_channel *iio_v;
+ struct iio_channel *iio_i;
+ struct gpio_desc *rechargeable_gpio;
+ struct power_supply *psy;
+ int technology;
+ int v_max;
+ int v_min;
+};
+
+static int lego_ev3_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct lego_ev3_battery *batt = power_supply_get_drvdata(psy);
+ int val2;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = batt->technology;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ /* battery voltage is iio channel * 2 + Vce of transistor */
+ iio_read_channel_processed(batt->iio_v, &val->intval);
+ val->intval *= 2000;
+ val->intval += 200000;
+ /* plus adjust for shunt resistor drop */
+ iio_read_channel_processed(batt->iio_i, &val2);
+ val2 *= 1000;
+ val2 /= 15;
+ val->intval += val2;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN:
+ val->intval = batt->v_max;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+ val->intval = batt->v_min;
+ break;
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
+ /* battery current is iio channel / 15 / 0.05 ohms */
+ iio_read_channel_processed(batt->iio_i, &val->intval);
+ val->intval *= 20000;
+ val->intval /= 15;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int lego_ev3_battery_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct lego_ev3_battery *batt = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ /*
+ * Only allow changing technology from Unknown to NiMH. Li-ion
+ * batteries are automatically detected and should not be
+ * overridden. Rechargeable AA batteries, on the other hand,
+ * cannot be automatically detected, and so must be manually
+ * specified. This should only be set once during system init,
+ * so there is no mechanism to go back to Unknown.
+ */
+ if (batt->technology != POWER_SUPPLY_TECHNOLOGY_UNKNOWN)
+ return -EINVAL;
+ switch (val->intval) {
+ case POWER_SUPPLY_TECHNOLOGY_NiMH:
+ batt->technology = POWER_SUPPLY_TECHNOLOGY_NiMH;
+ batt->v_max = 7800000;
+ batt->v_min = 5400000;
+ break;
+ default:
+ return -EINVAL;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int lego_ev3_battery_property_is_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ struct lego_ev3_battery *batt = power_supply_get_drvdata(psy);
+
+ return psp == POWER_SUPPLY_PROP_TECHNOLOGY &&
+ batt->technology == POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
+}
+
+static enum power_supply_property lego_ev3_battery_props[] = {
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_SCOPE,
+};
+
+static const struct power_supply_desc lego_ev3_battery_desc = {
+ .name = "lego-ev3-battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = lego_ev3_battery_props,
+ .num_properties = ARRAY_SIZE(lego_ev3_battery_props),
+ .get_property = lego_ev3_battery_get_property,
+ .set_property = lego_ev3_battery_set_property,
+ .property_is_writeable = lego_ev3_battery_property_is_writeable,
+};
+
+static int lego_ev3_battery_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct lego_ev3_battery *batt;
+ struct power_supply_config psy_cfg = {};
+ int err;
+
+ batt = devm_kzalloc(dev, sizeof(*batt), GFP_KERNEL);
+ if (!batt)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, batt);
+
+ batt->iio_v = devm_iio_channel_get(dev, "voltage");
+ err = PTR_ERR_OR_ZERO(batt->iio_v);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get voltage iio channel\n");
+ return err;
+ }
+
+ batt->iio_i = devm_iio_channel_get(dev, "current");
+ err = PTR_ERR_OR_ZERO(batt->iio_i);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get current iio channel\n");
+ return err;
+ }
+
+ batt->rechargeable_gpio = devm_gpiod_get(dev, "rechargeable", GPIOD_IN);
+ err = PTR_ERR_OR_ZERO(batt->rechargeable_gpio);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get rechargeable gpio\n");
+ return err;
+ }
+
+ /*
+ * The rechargeable battery indication switch cannot be changed without
+ * removing the battery, so we only need to read it once.
+ */
+ if (gpiod_get_value(batt->rechargeable_gpio)) {
+ /* 2-cell Li-ion, 7.4V nominal */
+ batt->technology = POWER_SUPPLY_TECHNOLOGY_LION;
+ batt->v_max = 84000000;
+ batt->v_min = 60000000;
+ } else {
+ /* 6x AA Alkaline, 9V nominal */
+ batt->technology = POWER_SUPPLY_TECHNOLOGY_UNKNOWN;
+ batt->v_max = 90000000;
+ batt->v_min = 48000000;
+ }
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = batt;
+
+ batt->psy = devm_power_supply_register(dev, &lego_ev3_battery_desc,
+ &psy_cfg);
+ err = PTR_ERR_OR_ZERO(batt->psy);
+ if (err) {
+ dev_err(dev, "failed to register power supply\n");
+ return err;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id of_lego_ev3_battery_match[] = {
+ { .compatible = "lego,ev3-battery", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, of_lego_ev3_battery_match);
+
+static struct platform_driver lego_ev3_battery_driver = {
+ .driver = {
+ .name = "lego-ev3-battery",
+ .of_match_table = of_lego_ev3_battery_match,
+ },
+ .probe = lego_ev3_battery_probe,
+};
+module_platform_driver(lego_ev3_battery_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Lechner <david@lechnology.com>");
+MODULE_DESCRIPTION("LEGO MINDSTORMS EV3 Battery Driver");
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: power: supply: New bindings for LEGO MINDSTORMS EV3 battery
From: David Lechner @ 2017-04-11 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491944723-32174-1-git-send-email-david@lechnology.com>
This add a new device tree binding for LEGO MINDSTORMS EV3 battery. The EV3
has some built-in capability for monitoring the attached battery.
Signed-off-by: David Lechner <david@lechnology.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../bindings/power/supply/lego_ev3_battery.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/lego_ev3_battery.txt
diff --git a/Documentation/devicetree/bindings/power/supply/lego_ev3_battery.txt b/Documentation/devicetree/bindings/power/supply/lego_ev3_battery.txt
new file mode 100644
index 0000000..5485633
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/lego_ev3_battery.txt
@@ -0,0 +1,21 @@
+LEGO MINDSTORMS EV3 Battery
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+LEGO MINDSTORMS EV3 has some built-in capability for monitoring the battery.
+It uses 6 AA batteries or a special Li-ion rechargeable battery pack that is
+detected by a key switch in the battery compartment.
+
+Required properties:
+ - compatible: Must be "lego,ev3-battery"
+ - io-channels: phandles to analog inputs for reading voltage and current
+ - io-channel-names: Must be "voltage", "current"
+ - rechargeable-gpios: phandle to the rechargeable battery indication gpio
+
+Example:
+
+ battery {
+ compatible = "lego,ev3-battery";
+ io-channels = <&adc 4>, <&adc 3>;
+ io-channel-names = "voltage", "current";
+ rechargeable-gpios = <&gpio 136 GPIO_ACTIVE_LOW>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/3] LEGO MINDSTORMS EV3 Battery
From: David Lechner @ 2017-04-11 21:05 UTC (permalink / raw)
To: linux-arm-kernel
This series adds support for the LEGO MINDSTORMS EV3 battery.
v2 changes:
* use devm_power_supply_register()
David Lechner (3):
dt-bindings: power: supply: New bindings for LEGO MINDSTORMS EV3
battery
power: supply: New driver for LEGO MINDSTORMS EV3 battery
ARM: dts: da850: Add node for LEGO MINDSTORMS EV3 Battery
.../bindings/power/supply/lego_ev3_battery.txt | 21 ++
arch/arm/boot/dts/da850-lego-ev3.dts | 28 +++
drivers/power/supply/Kconfig | 6 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/lego_ev3_battery.c | 228 +++++++++++++++++++++
5 files changed, 284 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/lego_ev3_battery.txt
create mode 100644 drivers/power/supply/lego_ev3_battery.c
--
2.7.4
^ permalink raw reply
* [PATCH v7 resend 00/20] ILP32 for ARM64
From: Yury Norov @ 2017-04-11 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <18edebeb-201e-a9d6-7e66-6e34f98a40df@redhat.com>
On Tue, Apr 11, 2017 at 08:42:24PM +0200, Florian Weimer wrote:
> On 04/11/2017 08:36 PM, Yury Norov wrote:
> >>Also, the latest benchmarks I've seen were mostly for user space
> >>while I'm more concerned with the user-kernel interface
> >>(https://marc.info/?l=linux-arm-kernel&m=148690490713310&w=2).
> >
> >>On the glibc testing side, have the regressions been identified/fixed?
> >
> >I run LTP for testing the ABI and kernel, and there is no failures in
> >ltplite scenario. With glibc testsuite, there's only 3 failures
> >comparing to lp64. (Steve, fix me if something changed.) This is
> >slides on ilp32 from Linaro Connect, hope you'll find it useful.
> >
> >https://docs.google.com/presentation/d/1TKZqgH0XJUgMMGkw2fJA3Lzr57slht1sGKYJVBJTNM4/edit?usp=sharing
>
> The listed failures are:
>
> misc/tst-sync_file_range
> nptl/tst-stack4
> malloc/tst-mallocstate
>
> If necessary, I will fix malloc/tst-mallocstate once there's support for a
> new architecture in build-many-glibcs.py. The failure is
> architecture-independent, it's related to the lack of a compat symbol and
> the difficulty of checking for that at the Makefile or test level.
>
> nptl/tst-stack4 is also a generic failure, I think.
That would be great, thanks.
> misc/tst-sync_file_range is probably a real failure related to argument
> passing. I think this system call was problematic on other architectures,
> too.
At first glance, it's pretty trivial, both on glibc and kernel side:
GLIBC:
int
sync_file_range (int fd, __off64_t offset, __off64_t len, unsigned int flags)
{
#if defined (__NR_sync_file_range2)
return SYSCALL_CANCEL (sync_file_range2, fd, flags, SYSCALL_LL64 (offset),
SYSCALL_LL64 (len));
#elif defined (__NR_sync_file_range)
return SYSCALL_CANCEL (sync_file_range, fd,
__ALIGNMENT_ARG SYSCALL_LL64 (offset),
SYSCALL_LL64 (len), flags);
#endif
}
And kernel:
ENTRY(compat_sys_sync_file_range2_wrapper)
regs_to_64 x2, x2, x3
regs_to_64 x3, x4, x5
b sys_sync_file_range2
ENDPROC(compat_sys_sync_file_range2_wrapper)
Anyway, I'll check everything and report here.
Yury
^ permalink raw reply
* [PATCH v2 1/5] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
From: Heiko Stübner @ 2017-04-11 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <936a53ee-2ed8-11e4-a860-744256676af9@collabora.com>
Hi Guillaume,
Am Dienstag, 11. April 2017, 18:40:37 CEST schrieb Guillaume Tucker:
> On 03/04/17 09:12, Neil Armstrong wrote:
> > On 04/02/2017 09:59 AM, Guillaume Tucker wrote:
> >> +Optional:
> >> +
> >> +- clocks : Phandle to clock for the Mali Midgard device.
> >> +- clock-names : Shall be "clk_mali".
> >> +- mali-supply : Phandle to regulator for the Mali device. Refer to
> >> + Documentation/devicetree/bindings/regulator/regulator.txt for details.
> >> +- operating-points : Refer to
> >> Documentation/devicetree/bindings/power/opp.txt + for details.
> >
> > Please add :
> > * Must be one of the following:
> > "arm,mali-t820"
> >
> > * And, optionally, one of the vendor specific compatible:
> > "amlogic,meson-gxm-mali"
> >
> > with my Ack for the amlogic platform.
>
> It seems to me that as long as the GPU architecture hasn't been
> modified (I don't think I've ever encountered such a case) then
> it has to be a standard ARM Mali type regardless of the SoC
> vendor. So unless a Mali-T820 in the Amlogic S912 SoC is not the
> same as a T820 in a different SoC, please forgive me but I don't
> understand why a vendor compatible string is needed. My main
> concern is that it's going to be very hard to keep that list
> up-to-date with all existing Midgard SoC variants. If do we need
> to add vendor compatible strings to correctly describe the
> hardware then I'm happy to add the amlogic one in my patch v3; I
> would just like to understand why that's necessary.
SoC vendors in most cases hook ip blocks into their socs in different
and often strange ways. After all it's not some discrete ic you solder
onto a board, but instead a part of the soc itself.
So in most cases you will have some hooks outside the actual gpu iospace
that can be used to tune different things about how the gpu interacts with
the system. Which is probably also the reason the midgard kernel driver
has this ugly "platform" subdirectory for compile-time platform selection.
On my rk3288 for example we have [0] in the chromeos tree, that handles
the oddities of the midgard on the rk3288 used in a lot of Chromebooks.
There are soc-specific oddities of frequencies, frequency-scaling and
whatnot. And there are also more gpu-specific setting in syscon areas
of the soc (pmu and grf) that can also influence the gpus performance
and might need tweaking at some point.
That doesn't even take into account that there may even be differences
on how things are synthesized that we don't know about. See all the
variants of the dw_hdmi ip block (imx, rockchip, meson [more?]) .
So we really want to have the special compatibles in place, to be prepared
for the future per-soc oddities that always appear :-) .
Heiko
[0] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.14/drivers/gpu/arm/midgard/platform/rk/
^ permalink raw reply
* [PATCH v5 04/10] drm/stm: Add STM32 LTDC driver
From: Daniel Vetter @ 2017-04-11 20:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87wpaqeks2.fsf@eliezer.anholt.net>
On Tue, Apr 11, 2017 at 01:45:01PM -0700, Eric Anholt wrote:
> Yannick Fertre <yannick.fertre@st.com> writes:
> > +static void ltdc_crtc_disable(struct drm_crtc *crtc)
> > +{
> > + struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > + struct drm_pending_vblank_event *event = crtc->state->event;
> > +
> > + DRM_DEBUG_DRIVER("\n");
> > +
> > + if (!crtc->enabled) {
> > + DRM_DEBUG_DRIVER("already disabled\n");
> > + return;
> > + }
>
> I think this crtc->enabled is a given for the disable() being called.
Yup, one design principle of atomic (compared to the legacy modeset
helpers) is to correctly keep track of hw state and not call a hook when
not needed. If you don't trust them, conver them to WARN_ON, but otherwise
best to remove.
But like Eric said, totally fine in a follow-up patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* [PATCH v5 04/10] drm/stm: Add STM32 LTDC driver
From: Eric Anholt @ 2017-04-11 20:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1490694293-18358-5-git-send-email-yannick.fertre@st.com>
Yannick Fertre <yannick.fertre@st.com> writes:
> This controller provides output signals to interface directly a variety
> of LCD and TFT panels. These output signals are: RGB signals
> (up to 24bpp), vertical & horizontal synchronisations, data enable and
> the pixel clock.
>
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
> ---
> drivers/gpu/drm/Kconfig | 3 +-
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/stm/Kconfig | 16 +
> drivers/gpu/drm/stm/Makefile | 7 +
> drivers/gpu/drm/stm/drv.c | 221 ++++++++
> drivers/gpu/drm/stm/ltdc.c | 1210 ++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/stm/ltdc.h | 40 ++
> 7 files changed, 1497 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/stm/Kconfig
> create mode 100644 drivers/gpu/drm/stm/Makefile
> create mode 100644 drivers/gpu/drm/stm/drv.c
> create mode 100644 drivers/gpu/drm/stm/ltdc.c
> create mode 100644 drivers/gpu/drm/stm/ltdc.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 78d7fc0..dd5762a 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -203,7 +203,6 @@ config DRM_VGEM
> as used by Mesa's software renderer for enhanced performance.
> If M is selected the module will be called vgem.
>
> -
Stray whitespace change.
With this removed, the driver is:
Reviewed-by: Eric Anholt <eric@anholt.net>
Apologies for the delay in the second review offered. The remainder of
my comments are little cleanups, all of which I think are optional and
fine to do after the code lands.
You should probably update MAINTAINERS for your new driver. If you'd
like to maintain this driver in the drm-misc small drivers collection
(https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html),
send a follow-up patch to the list to add the MAINTAINERS entry, and I
can get that and patches 1-4 merged. Once you have a few more patches
in, we can add you to the drm-misc committers crew so you can merge
directly after getting review.
I'll also take this moment to plug something: Please feel welcome to
review other people's driver patches on the list. You've built
something nice here, and probably learned a lot of lessons along the way
that you could share with others. (I just found out about
of_reset_control in reviewing your code, and I wish I had known about it
back when I was landing vc4!)
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> new file mode 100644
> index 0000000..922f021
> --- /dev/null
> +++ b/drivers/gpu/drm/stm/ltdc.c
> +static void ltdc_crtc_disable(struct drm_crtc *crtc)
> +{
> + struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> + struct drm_pending_vblank_event *event = crtc->state->event;
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> + if (!crtc->enabled) {
> + DRM_DEBUG_DRIVER("already disabled\n");
> + return;
> + }
I think this crtc->enabled is a given for the disable() being called.
> +
> + drm_crtc_vblank_off(crtc);
> +
> + /* disable LTDC */
> + reg_clear(ldev->regs, LTDC_GCR, GCR_LTDCEN);
> +
> + /* disable IRQ */
> + reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
> +
> + /* immediately commit disable of layers before switching off LTDC */
> + reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
> +
> + if (event) {
> + crtc->state->event = NULL;
> +
> + spin_lock_irq(&crtc->dev->event_lock);
> + if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0)
> + drm_crtc_arm_vblank_event(crtc, event);
> + else
> + drm_crtc_send_vblank_event(crtc, event);
> + spin_unlock_irq(&crtc->dev->event_lock);
> + }
I believe that we're guaranteed that crtc->state->event is NULL in the
disable call, since your atomic_flush() already armed or sent the event
and NULLed out the pointer.
> +struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev)
> +{
> + struct drm_connector *connector;
> + int err;
> +
> + connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
> + if (!connector) {
> + DRM_ERROR("Failed to allocate connector\n");
> + return NULL;
> + }
> +
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + err = drm_connector_init(ddev, connector, <dc_rgb_connector_funcs,
> + DRM_MODE_CONNECTOR_LVDS);
I think DRM_MODE_CONNECTOR_DPI (and _ENCODER_DPI) are slightly more
accurate descriptions, if I'm interpreting your pinmux setup right.
It's cosmetic, though.
> +static struct drm_panel *ltdc_get_panel(struct drm_device *ddev)
> +{
> + struct device *dev = ddev->dev;
> + struct device_node *np = dev->of_node;
> + struct device_node *entity, *port = NULL;
> + struct drm_panel *panel = NULL;
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> + /*
> + * Parse ltdc node to get remote port and find RGB panel / HDMI slave
> + * If a dsi or a bridge (hdmi, lvds...) is connected to ltdc,
> + * a remote port & RGB panel will not be found.
> + */
> + for_each_endpoint_of_node(np, entity) {
> + if (!of_device_is_available(entity))
> + continue;
> +
> + port = of_graph_get_remote_port_parent(entity);
> + if (port) {
> + panel = of_drm_find_panel(port);
> + of_node_put(port);
> + if (panel) {
> + DRM_DEBUG_DRIVER("remote panel %s\n",
> + port->full_name);
> + } else {
> + DRM_DEBUG_DRIVER("panel missing\n");
> + of_node_put(entity);
> + }
> + }
> + }
Future work: You may find the new drm_of_find_panel_or_bridge() useful
to drop this loop.
> +
> + return panel;
> +}
> +
> +int ltdc_load(struct drm_device *ddev)
> +{
> + struct platform_device *pdev = to_platform_device(ddev->dev);
> + struct ltdc_device *ldev = ddev->dev_private;
> + struct device *dev = ddev->dev;
> + struct device_node *np = dev->of_node;
> + struct drm_encoder *encoder;
> + struct drm_connector *connector = NULL;
> + struct drm_crtc *crtc;
> + struct reset_control *rstc;
> + struct resource res;
> + int irq, ret, i;
> +
> + DRM_DEBUG_DRIVER("\n");
> +
> + ldev->panel = ltdc_get_panel(ddev);
> + if (!ldev->panel)
> + return -EPROBE_DEFER;
> +
> + rstc = of_reset_control_get(np, NULL);
> +
> + mutex_init(&ldev->err_lock);
> +
> + ldev->pixel_clk = devm_clk_get(dev, "lcd");
> + if (IS_ERR(ldev->pixel_clk)) {
> + DRM_ERROR("Unable to get lcd clock\n");
> + return -ENODEV;
> + }
> +
> + if (clk_prepare_enable(ldev->pixel_clk)) {
> + DRM_ERROR("Unable to prepare pixel clock\n");
> + return -ENODEV;
> + }
Future work: You may want to move the pixel clock enable into the CRTC's
.enable() and disable in .disable(). It sounded in previous versions
like the HW uses that clock for all register accesses, so you'd need to
protect a couple of other places, but that should save power when the
device is off, right?
> +
> + if (of_address_to_resource(np, 0, &res)) {
> + DRM_ERROR("Unable to get resource\n");
> + return -ENODEV;
> + }
> +
> + ldev->regs = devm_ioremap_resource(dev, &res);
> + if (IS_ERR(ldev->regs)) {
> + DRM_ERROR("Unable to get ltdc registers\n");
> + return PTR_ERR(ldev->regs);
> + }
> +
> + for (i = 0; i < MAX_IRQ; i++) {
> + irq = platform_get_irq(pdev, i);
> + if (irq < 0)
> + continue;
> +
> + ret = devm_request_threaded_irq(dev, irq, ltdc_irq,
> + ltdc_irq_thread, IRQF_ONESHOT,
> + dev_name(dev), ddev);
> + if (ret) {
> + DRM_ERROR("Failed to register LTDC interrupt\n");
> + return ret;
> + }
> + }
> +
> + if (!IS_ERR(rstc))
> + reset_control_deassert(rstc);
> +
> + /* Disable interrupts */
> + reg_clear(ldev->regs, LTDC_IER,
> + IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
> +
> + ret = ltdc_get_caps(ddev);
> + if (ret) {
> + DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
> + ldev->caps.hw_version);
> + return ret;
> + }
> +
> + DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
> +
> + if (ltdc_create_encoders(ddev)) {
> + DRM_ERROR("Failed to create encoders\n");
> + return -EINVAL;
> + }
> +
> + if (ldev->panel) {
> + encoder = ltdc_rgb_encoder_find(ddev);
> + if (!encoder) {
> + DRM_ERROR("Failed to find RGB encoder\n");
> + ret = -EINVAL;
> + goto err;
> + }
Given that ltdc_create_encoders() only does work if ldev->panel, its
body could probably be moved in here and then ltdc_rgb_encoder_find
could be dropped.
> +
> + connector = ltdc_rgb_connector_create(ddev);
> + if (!connector) {
> + DRM_ERROR("Failed to create RGB connector\n");
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = drm_mode_connector_attach_encoder(connector, encoder);
> + if (ret) {
> + DRM_ERROR("Failed to attach connector to encoder\n");
> + goto err;
> + }
> +
> + drm_panel_attach(ldev->panel, connector);
> + }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/9b18b4a4/attachment.sig>
^ permalink raw reply
* Applied "regulator: anatop: check return value of of_get_regulator_init_data" to the regulator tree
From: Mark Brown @ 2017-04-11 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
The patch
regulator: anatop: check return value of of_get_regulator_init_data
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 7f51cf2ea7186e3f217e616a5522f1156678356f Mon Sep 17 00:00:00 2001
From: Dong Aisheng <aisheng.dong@nxp.com>
Date: Wed, 12 Apr 2017 09:58:42 +0800
Subject: [PATCH] regulator: anatop: check return value of
of_get_regulator_init_data
Should check the return value of of_get_regulator_init_data before
using it.
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/anatop-regulator.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 3a6d0290c54c..aa93f462ac6e 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -200,6 +200,9 @@ static int anatop_regulator_probe(struct platform_device *pdev)
rdesc->owner = THIS_MODULE;
initdata = of_get_regulator_init_data(dev, np, rdesc);
+ if (!initdata)
+ return -ENOMEM;
+
initdata->supply_regulator = "vin";
sreg->initdata = initdata;
--
2.11.0
^ permalink raw reply related
* Applied "regulator: anatop: use of_property_read_string to read the name" to the regulator tree
From: Mark Brown @ 2017-04-11 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-3-git-send-email-aisheng.dong@nxp.com>
The patch
regulator: anatop: use of_property_read_string to read the name
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 5062e04711dbc4f67b24ffd926cc67060267792d Mon Sep 17 00:00:00 2001
From: Dong Aisheng <aisheng.dong@nxp.com>
Date: Wed, 12 Apr 2017 09:58:44 +0800
Subject: [PATCH] regulator: anatop: use of_property_read_string to read the
name
sreg->name is a string, so use a more proper api to read back the string
instead of of_get_property.
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/anatop-regulator.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index aa93f462ac6e..58141cbdf257 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -193,7 +193,8 @@ static int anatop_regulator_probe(struct platform_device *pdev)
sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL);
if (!sreg)
return -ENOMEM;
- sreg->name = of_get_property(np, "regulator-name", NULL);
+
+ of_property_read_string(np, "regulator-name", &sreg->name);
rdesc = &sreg->rdesc;
rdesc->name = sreg->name;
rdesc->type = REGULATOR_VOLTAGE;
--
2.11.0
^ permalink raw reply related
* Applied "regulator: anatop: remove unneeded name field of struct anatop_regulator" to the regulator tree
From: Mark Brown @ 2017-04-11 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-4-git-send-email-aisheng.dong@nxp.com>
The patch
regulator: anatop: remove unneeded name field of struct anatop_regulator
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From aeb1404d68df62b0a1d277a4138dbd92a4330304 Mon Sep 17 00:00:00 2001
From: Dong Aisheng <aisheng.dong@nxp.com>
Date: Wed, 12 Apr 2017 09:58:45 +0800
Subject: [PATCH] regulator: anatop: remove unneeded name field of struct
anatop_regulator
sreg->name is only used as an intermediate assign of rdesc->name, plus
another strcmp. Since we already have rdesc->name, no need it anymore.
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/anatop-regulator.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 58141cbdf257..c6ce9745ffc8 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -39,7 +39,6 @@
#define LDO_FET_FULL_ON 0x1f
struct anatop_regulator {
- const char *name;
u32 control_reg;
struct regmap *anatop;
int vol_bit_shift;
@@ -194,12 +193,12 @@ static int anatop_regulator_probe(struct platform_device *pdev)
if (!sreg)
return -ENOMEM;
- of_property_read_string(np, "regulator-name", &sreg->name);
rdesc = &sreg->rdesc;
- rdesc->name = sreg->name;
rdesc->type = REGULATOR_VOLTAGE;
rdesc->owner = THIS_MODULE;
+ of_property_read_string(np, "regulator-name", &rdesc->name);
+
initdata = of_get_regulator_init_data(dev, np, rdesc);
if (!initdata)
return -ENOMEM;
@@ -297,7 +296,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
* a sane default until imx6-cpufreq was probed and changes the
* voltage to the correct value. In this case we set 1.25V.
*/
- if (!sreg->sel && !strcmp(sreg->name, "vddpu"))
+ if (!sreg->sel && !strcmp(rdesc->name, "vddpu"))
sreg->sel = 22;
if (!sreg->bypass && !sreg->sel) {
--
2.11.0
^ permalink raw reply related
* [PATCH 6/6] regulator: anatop: set default voltage selector for pcie
From: Mark Brown @ 2017-04-11 20:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-6-git-send-email-aisheng.dong@nxp.com>
On Wed, Apr 12, 2017 at 09:58:47AM +0800, Dong Aisheng wrote:
> Set the initial voltage selector for vddpcie in case it's disabled
> by default.
Why is this the only anatop regulator which can have this problem and
how do we know this is a good value?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/4f19af2e/attachment.sig>
^ permalink raw reply
* [PATCH 5/6] regulator: anatop-regulator: make regulator-name using optionally
From: Mark Brown @ 2017-04-11 20:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-5-git-send-email-aisheng.dong@nxp.com>
On Wed, Apr 12, 2017 at 09:58:46AM +0800, Dong Aisheng wrote:
> rdesc->name/regulator-name is optional according to standard regulator
> binding doc. Use it conditionally to avoid a kernel NULL point crash.
It is optional in the standard binding because it is used to override
the name statically provided in the driver for the device. Since the
anatop regulator is completely dynamic (there's no static list of
regulators in the device) it's mandatory for anatop regulators - you
should improve the error handling instead to detect a missing name.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/ab02c8e2/attachment.sig>
^ permalink raw reply
* [PATCH 2/6] regulator: anatop: only set supply regulator when it actually exists
From: Mark Brown @ 2017-04-11 20:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-2-git-send-email-aisheng.dong@nxp.com>
On Wed, Apr 12, 2017 at 09:58:43AM +0800, Dong Aisheng wrote:
> Mandatorily set the initdata->supply_regulator while it actually not
> exist will cause regulator core to resolve supply each time whenever
> a new regulator registered which is meaningless and waste CPU mips.
>
> We can observe more than one hundred times of iteration of resolving
> during a MX6Q SDB board booting up.
>
> This patch adds the condition check for vin-supply to avoid the issue.
This is an obvious abstraction failure - there is nothing magical about
your driver which means that we need special casing in it to handle
badly written DTs that don't specify supplies. Exactly the same
argument applies to all other regulators so if this is worth fixing it's
worth fixing in the core so we substitute in a dummy regulator if the
supply is genuinely missing. Which is something we in fact have code to
do already though for some reason I can't see we bypass it, I'll send a
patch just now...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/ff6b4591/attachment.sig>
^ permalink raw reply
* at91: serial: Can't use DMA on 4.x kernels in RS-485 mode on at91sam9x5
From: Alexandre Belloni @ 2017-04-11 20:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <DM5PR05MB295463261205406FFF273D3BBB000@DM5PR05MB2954.namprd05.prod.outlook.com>
On 11/04/2017 at 20:13:19 +0000, Bryan Evenson wrote:
> I checked the various branches on the linux-stable repository, and I see this commit in master and on 4.9, but I don't see it in any branches prior to that one. Since this issue was introduced back in 4.1, I think this patch would be suitable for backporting to the 4.1 and 4.4 stable branches. Is there something I can do to help make that happen?
>
Indeed, the Fixes: tag doesn't have the usual format, that would explain
why.
I'll resend to stable
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 1/2] dt-bindings: Document STM32 I2S bindings
From: Mark Brown @ 2017-04-11 20:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_Jsq+bsh8Byow4poT7cdpLY_skb7QxbzkAmj4q7Cf6Z88GWA@mail.gmail.com>
On Tue, Apr 11, 2017 at 11:02:57AM -0500, Rob Herring wrote:
> On Tue, Apr 11, 2017 at 9:32 AM, Mark Brown <broonie@kernel.org> wrote:
> > The graph card is blocked on your review...
> Maybe if there were more reviewers it would move faster. I don't know
> ASoC that well.
The times I've looked at it recently it's been stuck in DT style issues
rather than anything substantially ASoC related, as far as I can tell
the binding is essentially empty from an ASoC point of view and
inherited from the of_graph binding. There's bits in that are a bit
random like specifically listing the CPU DAIs and only them but that
just looks like one of these random DT things that's predetermined. I
really can't see anything at all in there to review from an ASoC point
of view, I've applied the changes that don't seem blocked on the
binding. If there's something you're looking for then please say...
We never seem to make any progress on the generic changes in drivers/of at
the start of the series either...
> > I'm also not clear how
> > without something like this we'd be able to identify a specific DAI
> > within a device if we don't have a way of identifying them.
> Isn't that what the graph does? "dais" points to a list of ports which
> are the specific DAIs whether there are multiple ones in a single
> device or multiple devices with a single DAI each.
But the ports can still have indexes AFAICT (the examples show port at 0,
port at 1 and so on) so we still need to define what those indexes mean
which is what this is doing?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170411/43cb8efb/attachment-0001.sig>
^ permalink raw reply
* at91: serial: Can't use DMA on 4.x kernels in RS-485 mode on at91sam9x5
From: Bryan Evenson @ 2017-04-11 20:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411185924.6g4cgk4h6ogsmyd5@piout.net>
Alexandre,
> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni at free-electrons.com]
> Sent: Tuesday, April 11, 2017 2:59 PM
> To: Bryan Evenson <bevenson@melinkcorp.com>
> Cc: Sylvain Rochet <sylvain.rochet@finsecur.com>; Nicolas Ferre
> <nicolas.ferre@atmel.com>; linux-arm-kernel at lists.infradead.org
> Subject: Re: at91: serial: Can't use DMA on 4.x kernels in RS-485 mode on
> at91sam9x5
>
> Hi,
>
> On 11/04/2017 at 18:32:20 +0000, Bryan Evenson wrote:
> > I narrowed my issues down to this commit:
> https://github.com/linux4sam/linux-
> at91/commit/0058f0871efe7b01c6f2b3046c68196ab73e96da
> >
> > tty/serial: atmel: fix RS485 half duplex with DMA
> >
> > When using DMA, half duplex doesn't work properly because rx is
> not stopped
> > before starting tx. Ensure we call atmel_stop_rx() in the DMA case.
> >
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-
> electrons.com>
> > Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Cc: stable <stable@vger.kernel.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > If I revert this commit, I can use both TX and RX DMA on all three RS-485
> ports (USART0, USART1 and USART3) with the current head of linux-4.4-at91
> and linux-4.1-at91 on Atmel's fork. I have not yet tried the mainline releases,
> but I see that atmel_start_tx() (the function modified by this commit) is
> identical in mainline so I'm assuming I'll have the same results. In my case I
> am using my RS-485 ports in half-duplex mode, so the call to atmel_stop_rx()
> inside of atmel_start_tx() is the cause of my problems.
> >
> > Anyone know why this would be a problem? The commit makes sense to
> me and I don't see a good reason why it would break things. I'm willing to
> test things out to see why this is causing a problem.
> >
>
> Do you have b389f173aaa1204d6dc1f299082a162eb0491545 in your tree ?
>
No, I did not have that commit in my tree. I cherry-picked from the linux-stable repository, tested and verified it fixed my DMA issues.
I checked the various branches on the linux-stable repository, and I see this commit in master and on 4.9, but I don't see it in any branches prior to that one. Since this issue was introduced back in 4.1, I think this patch would be suitable for backporting to the 4.1 and 4.4 stable branches. Is there something I can do to help make that happen?
Thanks,
Bryan
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
^ 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