* [BUG] FL1009: xHCI host not responding to stop endpoint command.
From: Sarah Sharp @ 2014-01-21 21:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87vbxhrmea.fsf@natisbad.org>
On Sat, Jan 18, 2014 at 10:49:17PM +0100, Arnaud Ebalard wrote:
> Hi,
>
> I have added Thomas in the recipients, because I guess he may be of some
> help debugging the issue further. Thomas, the beginning of the thread is
> here: http://thread.gmane.org/gmane.linux.usb.general/101531
...
> I started suspecting the introduction of MSI support in Marvell PCIe
> host controller driver (FL1009 is on the PCIe bus) and compiled a
> a 3.13.0-rc8 w/ CONFIG_PCI_MSI disabled (it was enabled in all my
> previous tests): I did not manage to reproduce the issue with this
> kernel. As a side note, commits 5b4deb6526bd, 31f614edb726 and
> 627dfcc249e2 are
>
> ATM, I do not know if the problem is related to a bug in introduced MSI
> support or some weird incompatibility of that functionality with the
> FL1009 which would require some quirk in XHCI stack.
We've actually had issues in the past with Fresco Logic hosts not
supporting MSI properly, even though the PCI devices claim to have MSI
support. So turning off CONFIG_PCI_MSI may actually mean the Fresco
Logic host is to blame, rather than the Marvell patches. I assume MSI
wouldn't have been turned on for the Fresco Logic host unless the parent
PCI host controller supported it.
Let's see if the Fresco Logic host is really the root cause. Please
apply the this patch to 3.13.0-rc8 and recompile with CONFIG_PCI_MSI
enabled:
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6c03584ac15f..74748444c040 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -30,6 +30,7 @@
/* Device for a quirk */
#define PCI_VENDOR_ID_FRESCO_LOGIC 0x1b73
#define PCI_DEVICE_ID_FRESCO_LOGIC_PDK 0x1000
+#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1009 0x1009
#define PCI_DEVICE_ID_FRESCO_LOGIC_FL1400 0x1400
#define PCI_VENDOR_ID_ETRON 0x1b6f
@@ -63,6 +64,9 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
/* Look for vendor-specific quirks */
if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
+ pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1009)
+ xhci->quirks |= XHCI_BROKEN_MSI;
+ if (pdev->vendor == PCI_VENDOR_ID_FRESCO_LOGIC &&
(pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK ||
pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_FL1400)) {
if (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK &&
Sarah Sharp
^ permalink raw reply related
* [PATCH 1/4] ARM: dts: cm-fx6: rearrange the dts file
From: Sascha Hauer @ 2014-01-21 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390320812-25100-2-git-send-email-grinberg@compulab.co.il>
Hi Igor,
On Tue, Jan 21, 2014 at 06:13:29PM +0200, Igor Grinberg wrote:
> Move the iomuxc definition before the devices definitions
> (e.g. gpmi, fec).
The nodes are sorted alphabetically. Please keep them sorted.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH 2/6] drm/armada: Use new drm debugfs file helper
From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390336402-2049-1-git-send-email-benjamin.widawsky@intel.com>
The debugfs helper duplicates the functionality used by Armada, so let's
just use that.
WARNING: Not even compile tested. I can compile test this if required,
but I figured there'd be other kind people who already have the cross
toolchain setup.
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/armada/armada_debugfs.c | 40 ++++-----------------------------
1 file changed, 4 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_debugfs.c b/drivers/gpu/drm/armada/armada_debugfs.c
index 471e456..64e4361 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -107,38 +107,6 @@ static struct drm_info_list armada_debugfs_list[] = {
};
#define ARMADA_DEBUGFS_ENTRIES ARRAY_SIZE(armada_debugfs_list)
-static int drm_add_fake_info_node(struct drm_minor *minor, struct dentry *ent,
- const void *key)
-{
- struct drm_info_node *node;
-
- node = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL);
- if (node == NULL) {
- debugfs_remove(ent);
- return -ENOMEM;
- }
-
- node->minor = minor;
- node->dent = ent;
- node->info_ent = (void *) key;
-
- mutex_lock(&minor->debugfs_lock);
- list_add(&node->list, &minor->debugfs_list);
- mutex_unlock(&minor->debugfs_lock);
-
- return 0;
-}
-
-static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor,
- const char *name, umode_t mode, const struct file_operations *fops)
-{
- struct dentry *de;
-
- de = debugfs_create_file(name, mode, root, minor->dev, fops);
-
- return drm_add_fake_info_node(minor, de, fops);
-}
-
int armada_drm_debugfs_init(struct drm_minor *minor)
{
int ret;
@@ -149,13 +117,13 @@ int armada_drm_debugfs_init(struct drm_minor *minor)
if (ret)
return ret;
- ret = armada_debugfs_create(minor->debugfs_root, minor,
- "reg", S_IFREG | S_IRUSR, &fops_reg_r);
+ ret = drm_debugfs_create_file(minor->debugfs_root, minor, "reg",
+ S_IFREG | S_IRUSR, &fops_reg_r);
if (ret)
goto err_1;
- ret = armada_debugfs_create(minor->debugfs_root, minor,
- "reg_wr", S_IFREG | S_IWUSR, &fops_reg_w);
+ ret = drm_debugfs_create_file(minor->debugfs_root, minor,
+ "reg_wr", S_IFREG | S_IWUSR, &fops_reg_w);
if (ret)
goto err_2;
return ret;
--
1.8.5.3
^ permalink raw reply related
* [PATCH 1/6] drm: Create a debugfs file creation helper
From: Ben Widawsky @ 2014-01-21 20:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389708847-25038-1-git-send-email-benjamin.widawsky@intel.com>
DRM layer already provides a helper function to create a set of files
following a specific idiom primarily characterized by the access flags
of the file, and the file operations. This is great for writing concise
code to handle these very common cases.
This new helper function is provided to drivers that wish to change
either the access flags, or the fops for the file - in particular the
latter. This usage has become cropping up over many of the drivers.
Providing the helper is therefore here for the usual reasons.
Upcoming patches will update the callers. Currently the key is the same
as fops. This is what most callers want, the exception is when you want
to use 1 fops for multiple file nodes. I have found one case for this
(in i915), and don't feel it's a good idea to have the interface reflect
this one, currently fringe usage. In the future if more callers want to
have a separate fops and key, the interface should be extended.
v2: The original patch simply changes the way in which we wedge special
files into the normal drm node list. Daniel requested a fuller helper
function, which this patch addresses. His original claim that v1 was
buggy is unfounded.
Requested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/drm_debugfs.c | 49 +++++++++++++++++++++++++++++++++++++++++++
include/drm/drmP.h | 14 +++++++++++++
2 files changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index b4b51d4..ea470b7 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -237,5 +237,54 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
return 0;
}
+/**
+ * Helper function for DRM drivers to create a debugfs file.
+ *
+ * \param root parent directory entry for the new file
+ * \param minor minor device minor number
+ * \param name the new file's name
+ * \param fops file operations for the new file
+ * \param mode permissions for access
+ * \return zero on success, negative errno otherwise
+ *
+ * Instantiate a debugfs file with aforementioned data. The file will be added
+ * to the drm debugfs file list in the standard way, thus making cleanup
+ * possible through the normal drm_debugfs_remove_files() call.
+ *
+ * The primary motivation for this interface over the existing one is this
+ * interface provides explicit mode, and fops flags at the cost of some extra
+ * bookkeeping to be done by the driver.
+ */
+int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode)
+{
+ struct drm_info_node *node;
+ struct dentry *ent;
+
+ ent = debugfs_create_file(name, mode, root, minor->dev, fops);
+ if (!ent)
+ return PTR_ERR(ent);
+
+ node = kmalloc(sizeof(*node), GFP_KERNEL);
+ if (node == NULL) {
+ debugfs_remove(ent);
+ return -ENOMEM;
+ }
+
+ node->minor = minor;
+ node->dent = ent;
+ node->info_ent = (void *)fops;
+
+ mutex_lock(&minor->debugfs_lock);
+ list_add(&node->list, &minor->debugfs_list);
+ mutex_unlock(&minor->debugfs_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_debugfs_create_file);
+
#endif /* CONFIG_DEBUG_FS */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 63eab2b..b8e2baf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1458,6 +1458,11 @@ extern struct drm_local_map *drm_getsarea(struct drm_device *dev);
#if defined(CONFIG_DEBUG_FS)
extern int drm_debugfs_init(struct drm_minor *minor, int minor_id,
struct dentry *root);
+extern int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode);
extern int drm_debugfs_create_files(const struct drm_info_list *files,
int count, struct dentry *root,
struct drm_minor *minor);
@@ -1478,6 +1483,15 @@ static inline int drm_debugfs_create_files(const struct drm_info_list *files,
return 0;
}
+static inline int drm_debugfs_create_file(struct dentry *root,
+ struct drm_minor *minor,
+ const char *name,
+ const struct file_operations *fops,
+ umode_t mode)
+{
+ return 0;
+}
+
static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
int count, struct drm_minor *minor)
{
--
1.8.5.3
^ permalink raw reply related
* ARM kernel (& ext3) stability issues with GCC 4.8?
From: Aaro Koskinen @ 2014-01-21 19:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130613220429.GC18360@blackmetal.musicnaut.iki.fi>
Hi,
On Fri, Jun 14, 2013 at 01:04:29AM +0300, Aaro Koskinen wrote:
> I'm having severe stability issues with 3.10-rc5 on OpenRD (kirkwood)
> when cross-compiling it using GCC 4.8.1 (I tried also GCC 4.8.0
> with 3.9-something earlier with pretty much same results).
[...]
> [ 3383.627778] ------------[ cut here ]------------
> [ 3383.654029] kernel BUG at fs/buffer.c:2956!
It looks like the issue was caused by GCC bug 58854
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854).
The bug is still present in GCC 4.8.2 but there's a fix available in GCC
repository. The bug resulted in at least fs/buffer.c getting compiled
incorrectly, and what we saw were kernel crashes or file system getting
corrupted or both.
A.
^ permalink raw reply
* [PATCH 1/2] ohci-platform: Add support for controllers with big-endian regs / descriptors
From: Florian Fainelli @ 2014-01-21 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <Pine.LNX.4.44L0.1401211308040.1272-100000@iolanthe.rowland.org>
2014/1/21 Alan Stern <stern@rowland.harvard.edu>:
> On Tue, 21 Jan 2014, Hans de Goede wrote:
>
>> Hi,
>>
>> On 01/21/2014 05:40 PM, Alan Stern wrote:
>> > On Tue, 21 Jan 2014, Hans de Goede wrote:
>> >
>> >> Note this commit uses the same devicetree booleans for this as the ones
>> >> already existing in the usb-ehci bindings.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> >
>> >> --- a/drivers/usb/host/Kconfig
>> >> +++ b/drivers/usb/host/Kconfig
>> >> @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI
>> >>
>> >> config USB_OHCI_HCD_PLATFORM
>> >> tristate "Generic OHCI driver for a platform device"
>> >> + # Always support LE, support BE on architectures which have readl_be
>> >> + select USB_OHCI_LITTLE_ENDIAN
>> >> + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>> >> + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>> >> default n
>
>> > In any case, the style we have adopted is that these select lines go in
>> > the arch-specific defconfig, not here.
>>
>> Ok, so I should drop the Kconfig parts of both patches ?
>
> That's rigt. They are likely to cause trouble, and if the selects were
> needed then they should already be present somewhere else.
Absolutely, they will actually break platforms. Since you added some
guards for when these properties are set, but proper support in the
kernel is not enabled, this is already catching misuses, and as such
is already an improvement.
--
Florian
^ permalink raw reply
* [PATCH] mmc: omap_hsmmc: Add support for Erratum 2.1.1.128 in device tree boot
From: Felipe Balbi @ 2014-01-21 19:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGo_u6qHFmgb9s9Taw2NAuBgjLY5tDW5XHbfik7PHcTArW7szg@mail.gmail.com>
On Tue, Jan 21, 2014 at 01:39:20PM -0600, Nishanth Menon wrote:
> On Tue, Jan 21, 2014 at 12:05 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Tue, Jan 21, 2014 at 11:38:00AM -0600, Nishanth Menon wrote:
> >> On 01/20/2014 05:39 PM, Felipe Balbi wrote:
> >> > On Mon, Jan 20, 2014 at 05:29:02PM -0600, Nishanth Menon wrote:
> >> >> When device is booted using devicetree, platforms impacted by
> >> >> Erratum 2.1.1.128 is not detected easily in the mmc driver. This erratum
> >> >> indicates that the module cannot do multi-block transfers.
> >> >>
> >> >> Handle this by providing a boolean flag to indicate to driver that it is
> >> >> working on a hardware with mentioned limitation.
> >> >
> >> > sure there's no way of reading the revision register to figure this one
> >> > out without having to add a new DT attribute ?
> >> >
> >> I did a quick patch to read the Module revision register:
> >> http://slexy.org/view/s21TKvlWlR
> >>
> >> sdp2430: Revision: 1.2, Spec: 0.0, normal interrupt
> >>
> >> OMAP3430-ldp: (ES2.1): Revision: 2.6, Spec: 0.0, normal interrupt
> >> SDP3430:(ES3.0) Revision: 2.6, Spec: 0.0, normal interrupt
> >> AM3517-evm: (ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
> >> AM3517-crane:(ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
> >>
> >> AM37x-evm: (ES1.2) Revision: 2.6, Spec: 0.0, normal interrupt
> >> OMAP3630-beag-xm (ES1.2): Revision: 2.6, Spec: 0.0, normal interrupt
> >>
> >> am335x-evm:(ES1.0): Revision: 3.1, Spec: 0.1, normal interrupt
> >> am335x-sk: (ES2.1): Revision: 3.1, Spec: 0.1, normal interrupt
> >> am335x-beaglebone-black:(ES2.0): Revision: 3.1, Spec: 0.1, normal
> >> interrupt
> >>
> >> sdp4430.txt: (ES2.2): Revision: 3.1, Spec: 0.1, normal interrupt
> >>
> >> OMAP4460-panda-es (ES1.1): Revision: 3.1, Spec: 0.1, normal interrupt
> >>
> >> OMAP5uevm:(ES2.0): Revision: 3.3, Spec: 0.2, normal interrupt
> >> dra7-evm (es1.1): Revision: 3.3, Spec: 0.2, normal interrupt
> >>
> >>
> >> OMAP3430-ldp seems to be the only one impacted with module revision
> >> 2.6 -> so using revision information is not really helpful here. Hence
> >> the usage of a flag in dt attribute to indicate hardware impacted by
> >> erratum.
> >
> > alright, that's too bad. Seems like revision in this module isn't very
> > useful :-(
>
> Can I take that as an acked-by?
sure
Acked-by: Felipe Balbi <balbi@ti.com>
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140121/c11cd052/attachment.sig>
^ permalink raw reply
* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Florian Fainelli @ 2014-01-21 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390319315-8391-2-git-send-email-hdegoede@redhat.com>
2014/1/21 Hans de Goede <hdegoede@redhat.com>:
> This uses the already documented devicetree booleans for this.
(I would greatly appreciate if you could CC people who gave you
feedback on this before)
A more informative commit message would be welcome, along with a
reference to which Device Tree binding documentation you are referring
to.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/usb/host/Kconfig | 3 +++
> drivers/usb/host/ehci-platform.c | 33 +++++++++++++++++++++++++++++++--
> 2 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 237d7b1..4af41f3 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
> config USB_EHCI_HCD_PLATFORM
> tristate "Generic EHCI driver for a platform device"
> depends on !PPC_OF
> + # Support BE on architectures which have readl_be
> + select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
> + select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
I do not think this is that simple nor correct for at least Microblaze
and MIPS since they can run in either BE or LE mode, and those
specific platforms should already do the proper select at the
board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
although I believe some specific PPC64 boards can run in little-endian
mode like the P-series, SPARC might too.
It seems to me that you should not touch this and keep the existing
selects in place, if it turns out that the selects are missing the
error messages you added below are catching those misuses.
> default n
> ---help---
> Adds an EHCI host driver for a generic platform device, which
> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> index d8aebc0..5888abb 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>
> hcd->has_tt = pdata->has_tt;
> ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
> - ehci->big_endian_desc = pdata->big_endian_desc;
> - ehci->big_endian_mmio = pdata->big_endian_mmio;
> + if (pdata->big_endian_desc)
> + ehci->big_endian_desc = 1;
> + if (pdata->big_endian_mmio)
> + ehci->big_endian_mmio = 1;
>
> if (pdata->pre_setup) {
> retval = pdata->pre_setup(hcd);
> @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device *dev)
> struct resource *res_mem;
> struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
> struct ehci_platform_priv *priv;
> + struct ehci_hcd *ehci;
> int err, irq, clk = 0;
>
> if (usb_disabled())
> @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device *dev)
> platform_set_drvdata(dev, hcd);
> dev->dev.platform_data = pdata;
> priv = hcd_to_ehci_priv(hcd);
> + ehci = hcd_to_ehci(hcd);
>
> if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
> + if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
> + ehci->big_endian_mmio = 1;
> +
> + if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
> + ehci->big_endian_desc = 1;
> +
> + if (of_property_read_bool(dev->dev.of_node, "big-endian"))
> + ehci->big_endian_mmio = ehci->big_endian_desc = 1;
Ok, so I am confused now, should you update
pdata->ehci_big_endian_{desc,mmio} here or is it valid to directly
modify ehci->big_endian_{desc,mmio}, is not there any risk to undo
what is done in ehci_platform_reset(), or is ehci_platform_reset()
only called for non-DT cases?
> +
> +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
> + if (ehci->big_endian_mmio) {
> + dev_err(&dev->dev,
> + "Error big-endian-regs not compiled in\n");
I do not think using the Device Tree property name would be very
informative since this is supposed to guard against misconfigurations
for both DT and non-DT enabled platforms, how about something like the
following:
"support for big-endian MMIO registers not enabled".
> + err = -EINVAL;
> + goto err_put_hcd;
> + }
> +#endif
> +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_DESC
> + if (ehci->big_endian_desc) {
> + dev_err(&dev->dev,
> + "Error big-endian-desc not compiled in\n");
> + err = -EINVAL;
> + goto err_put_hcd;
And here "support for big-endian descriptors not enabled".
> + }
> +#endif
> priv->phy = devm_phy_get(&dev->dev, "usb");
> if (IS_ERR(priv->phy)) {
> err = PTR_ERR(priv->phy);
> --
> 1.8.5.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Florian
^ permalink raw reply
* [PATCH] mmc: omap_hsmmc: Add support for Erratum 2.1.1.128 in device tree boot
From: Nishanth Menon @ 2014-01-21 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121180549.GS30451@saruman.home>
On Tue, Jan 21, 2014 at 12:05 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Tue, Jan 21, 2014 at 11:38:00AM -0600, Nishanth Menon wrote:
>> On 01/20/2014 05:39 PM, Felipe Balbi wrote:
>> > On Mon, Jan 20, 2014 at 05:29:02PM -0600, Nishanth Menon wrote:
>> >> When device is booted using devicetree, platforms impacted by
>> >> Erratum 2.1.1.128 is not detected easily in the mmc driver. This erratum
>> >> indicates that the module cannot do multi-block transfers.
>> >>
>> >> Handle this by providing a boolean flag to indicate to driver that it is
>> >> working on a hardware with mentioned limitation.
>> >
>> > sure there's no way of reading the revision register to figure this one
>> > out without having to add a new DT attribute ?
>> >
>> I did a quick patch to read the Module revision register:
>> http://slexy.org/view/s21TKvlWlR
>>
>> sdp2430: Revision: 1.2, Spec: 0.0, normal interrupt
>>
>> OMAP3430-ldp: (ES2.1): Revision: 2.6, Spec: 0.0, normal interrupt
>> SDP3430:(ES3.0) Revision: 2.6, Spec: 0.0, normal interrupt
>> AM3517-evm: (ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
>> AM3517-crane:(ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
>>
>> AM37x-evm: (ES1.2) Revision: 2.6, Spec: 0.0, normal interrupt
>> OMAP3630-beag-xm (ES1.2): Revision: 2.6, Spec: 0.0, normal interrupt
>>
>> am335x-evm:(ES1.0): Revision: 3.1, Spec: 0.1, normal interrupt
>> am335x-sk: (ES2.1): Revision: 3.1, Spec: 0.1, normal interrupt
>> am335x-beaglebone-black:(ES2.0): Revision: 3.1, Spec: 0.1, normal
>> interrupt
>>
>> sdp4430.txt: (ES2.2): Revision: 3.1, Spec: 0.1, normal interrupt
>>
>> OMAP4460-panda-es (ES1.1): Revision: 3.1, Spec: 0.1, normal interrupt
>>
>> OMAP5uevm:(ES2.0): Revision: 3.3, Spec: 0.2, normal interrupt
>> dra7-evm (es1.1): Revision: 3.3, Spec: 0.2, normal interrupt
>>
>>
>> OMAP3430-ldp seems to be the only one impacted with module revision
>> 2.6 -> so using revision information is not really helpful here. Hence
>> the usage of a flag in dt attribute to indicate hardware impacted by
>> erratum.
>
> alright, that's too bad. Seems like revision in this module isn't very
> useful :-(
Can I take that as an acked-by?
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Russell King - ARM Linux @ 2014-01-21 19:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5DP1axSmKcdp0e5yNakirG-_yQ1dHdoqzww3wp3CUt29w@mail.gmail.com>
On Mon, Jan 20, 2014 at 04:47:34PM -0200, Fabio Estevam wrote:
> On Mon, Jan 20, 2014 at 3:16 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>
> > Not as far as I can see. fixed-clock appears to have two properies:
> >
> > clock-frequency
> > clock-output-names
> >
> > and nothing else. See of_fixed_clk_setup in drivers/clk/clk-fixed-rate.c.
> > You'll also find that the documentation in this file says this about it:
>
> Looks like Documentation/devicetree/bindings/clock/fixed-clock.txt is
> misleading then:
>
> "Optional properties:
> - gpios : From common gpio binding; gpio connection to clock enable pin."
It seems so. It appears that the binding was added to clk-fixed-rate.c
here:
commit 015ba40246497ae02a5f644d4c8adfec76d9b75c
Date: Sat Apr 7 21:39:39 2012 -0500
and the binding documentation separately:
commit 766e6a4ec602d0c107553b91b3434fe9c03474f4
Date: Mon Apr 9 14:50:06 2012 -0500
>From what I can see, this "gpios" property has never been supported.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH RFC v2 REPOST 3/8] ASoC: davinci-evm: HDMI audio support for TDA998x trough McASP I2S bus
From: Mark Brown @ 2014-01-21 19:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D67099.1040904@ti.com>
On Wed, Jan 15, 2014 at 01:27:21PM +0200, Jyri Sarha wrote:
> On 12/31/2013 03:25 PM, Mark Brown wrote:
> >>support. The only supported sample format is SNDRV_PCM_FORMAT_S32_LE.
> >>The 8 least significant bits are ignored.
> >Where does this constraint come from?
> From driver/gpu/drm/i2c/tda998x_drv.c. The driver configures CTS_N
> register statically to a value that works only with 4 byte samples.
> According to my tests it is possible to support 3 and 2 byte samples
> too by changing the CTS_N register value, but I am not sure if the
> configuration can be changed on the fly. My data sheet of the nxp
> chip is very vague about the register definitions, but I suppose the
> register configures some clock divider on the chip. HDMI supports
> only upto 24bit audio and the data sheet states that any extraneous
> least significant bits are ignored.
Hrm. This sounds like it ought to be presenting as an ASoC CODEC
driver.
> >>+ snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS,
> >>+ 2, 2);
> >Why not just set all this statically when registering the DAI?
> Because there is no relevant DAI to where to put these limitations.
> I did not want to add yet another dummy codec driver, but decided to
> use the already existing ASoC HDMI codec. By default the driver
> support all audio params supported by HDMI. The limitations are
> coming from NXP chip, the NXP driver, and because the chip is used
> in i2s mode. In other words the limitation is coming from machine
> setup, not from the DAIs.
OK, but it sounds like it's got register configuration as well? That
really does sound like a device that ought to have a driver...
> No excuse for i initialization, I'll fix that. The casting is just
> to survive with just one kmalloc call instead of separate memory
> blobs for
> struct snd_pcm_hw_constraint_list and referred list of supported
> sample rates. I'll allocate a second blob, if that is easier to
> read.
Yes, please. Unless it's something where memory is getting tight (eg,
due to be allocated a lot) it's more important that we can read the code
than that we save a few bytes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140121/b1d094fe/attachment.sig>
^ permalink raw reply
* [Intel-gfx] [PATCH 1/2] drm: share drm_add_fake_info_node
From: Ben Widawsky @ 2014-01-21 19:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140115234222.GA4770@phenom.ffwll.local>
On Thu, Jan 16, 2014 at 12:42:22AM +0100, Daniel Vetter wrote:
> On Wed, Jan 15, 2014 at 12:08:19PM -0800, Ben Widawsky wrote:
> > On Wed, Jan 15, 2014 at 09:45:28AM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 15, 2014 at 9:39 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > > On Wed, Jan 15, 2014 at 12:40 AM, Russell King - ARM Linux
> > > > <linux@arm.linux.org.uk> wrote:
> > > >> On Wed, Jan 15, 2014 at 12:25:10AM +0100, Daniel Vetter wrote:
> > > >>> On Tue, Jan 14, 2014 at 06:14:06AM -0800, Ben Widawsky wrote:
> > > >>> > Both i915 and Armada had the exact same implementation. For an upcoming
> > > >>> > patch, I'd like to call this function from two different source files in
> > > >>> > i915, and having it available externally helps there too.
> > > >>> >
> > > >>> > While moving, add 'debugfs_' to the name in order to match the other drm
> > > >>> > debugfs helper functions.
> > > >>> >
> > > >>> > Cc: linux-arm-kernel at lists.infradead.org
> > > >>> > Cc: intel-gfx at lists.freedesktop.org
> > > >>> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > >>>
> > > >>> drm_debugfs_create_files in drm_debugfs.c has the almost same code again.
> > > >>> Now the problem here is that the interface is a bit botched up, since all
> > > >>> the users in i915 and armada actaully faile to clean up teh debugfs dentry
> > > >>> if drm_add_fake_info_node.
> > > >>
> > > >> That's not correct - armada does clean up these, I think you need to
> > > >> take a closer look at the code.
> > > >>
> > > >> We do this by setting node->info_ent to the file operations structure,
> > > >> which is a unique key to the file being registered.
> > > >>
> > > >> Upon failure to create the fake node, we appropriately call
> > > >> drm_debugfs_remove_files() with the first argument being a pointer to
> > > >> the file operations. This causes drm_debugfs_remove_files() to match
> > > >> the fake entry, call debugfs_remove() on the dentry, and remove the
> > > >> node from the list, and free the node structure we allocated.
> > > >>
> > > >> Upon driver teardown, we also call drm_debugfs_remove_files() but with
> > > >> each fops which should be registered, thus cleaning up each fake node
> > > >> which was created.
> > > >>
> > > >> So, Armada does clean up these entries properly.
> > > >
> > > > Indeed I've missed that and it's just i915 that botches this. I still
> > > > think the helper would be saner if it cleans up all its leftovers in
> > > > the failure case.
> > >
> > > Ok, now I've actually page all the stuff back in - if
> > > drm_add_fake_info_node fails we don't set up a drm_info_node structure
> > > and link it anywhere. Which means drm_debugfs_remove_files won't ever
> > > find it and hence can't possibly call debugfs_remove. Which means the
> > > debugfs dentry is leaked. So I think the semantics of that new debugfs
> > > helper should get fixed to also allocate and clean up the debugfs
> > > node.
> > >
> > > I agree that i915 is even worse since it doesn't bother to clean up
> > > any debugfs files at all in the failure case.
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> >
> > Perhaps I don't understand what you want here. The only failure path in
> > the fake entry creation does already call debugfs_remove.
> >
> > if (node == NULL) {
> > debugfs_remove(ent);
> > return -ENOMEM;
> > }
> >
> > So long as the function succeeds, the node will be findable and removable.
>
> Oh dear, I didn't see that. Still stand by my opinion though that we
> should shovel the debugfs_create_file into the helper - callers allocating
> something and then the helper freeing it (but only if it fails) is rather
> funky semantics.
> -Daniel
This actually falls apart for the one usage I originally cared about.
For CRC, we store our own info structure instead of fops as the key in
the drm debug list. Therefore, it doesn't align with the usage in ours
or other drivers. One option is to make the helper function take both a
fops as well as a key (an ugly interface if you ask me).
I've already written the patches as you wanted, so I can submit them -
it's just unfortunate that the duplication I was hoping to get rid of
will need to remain.
--
Ben Widawsky, Intel Open Source Technology Center
^ permalink raw reply
* [PATCH RFC 4/6] net: rfkill: gpio: add device tree support
From: Arnd Bergmann @ 2014-01-21 18:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAVeFuKFfAvSPYLmvWV5jjT-peZFJ8sJ2bbh4F=JAYoWLhjZpA@mail.gmail.com>
On Tuesday 21 January 2014, Alexandre Courbot wrote:
> >> As discussed earlier in this thread I'm not sure the con_id is
> >> suitable for labelling GPIOs. It'd be better to have a proper name
> >> specified in DT/ACPI instead.
> >
> > +1
>
> I wonder why you guys prefer to have the name defined in the GPIO
> mapping. Having the driver decide the label makes it easier to look up
> which GPIO does what in debugfs, whereas nothing prevents people to
> name GPIOs whatever inadequate name they want in the device DT node.
> What am I overlooking here?
I should have another look at the debugfs representation, but isn't
there a global namespace that gets used for all gpios? Neither the
con_id nor the name that the driver picks would be globally unique
and stable across kernel versions, so they don't make a good user
interface.
I think what we want here is a system-wide unique identifier for
each gpio line that may get represented in debugfs, and use a new
DT mechanism to communicate that.
Arnd
^ permalink raw reply
* [PATCH v2 5/7] ARM: perf_event: Fully support Krait CPU PMU events
From: Stephen Boyd @ 2014-01-21 18:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121180711.GN30706@mudshark.cambridge.arm.com>
On 01/21/14 10:07, Will Deacon wrote:
> Hi Stephen,
>
> Thanks for the updates. A few more comments inline.
>
> On Wed, Jan 15, 2014 at 05:55:33PM +0000, Stephen Boyd wrote:
>> Krait supports a set of performance monitor region event
>> selection registers (PMRESR) sitting behind a cp15 based
>> interface that extend the architected PMU events to include Krait
>> CPU and Venum VFP specific events. To use these events the user
>> is expected to program the region register (PMRESRn) with the
>> event code shifted into the group they care about and then point
>> the PMNx event at that region+group combo by writing a
>> PMRESRn_GROUPx event. Add support for this hardware.
>>
>> Note: the raw event number is a pure software construct that
>> allows us to map the multi-dimensional number space of regions,
>> groups, and event codes into a flat event number space suitable
>> for use by the perf framework.
> [...]
>
>> +static u32 krait_read_pmresrn(int n)
>> +{
>> + u32 val;
>> +
>> + switch (n) {
>> + case 0:
>> + asm volatile("mrc p15, 1, %0, c9, c15, 0" : "=r" (val));
>> + break;
>> + case 1:
>> + asm volatile("mrc p15, 1, %0, c9, c15, 1" : "=r" (val));
>> + break;
>> + case 2:
>> + asm volatile("mrc p15, 1, %0, c9, c15, 2" : "=r" (val));
>> + break;
>> + default:
>> + BUG(); /* Should be validated in krait_pmu_get_event_idx() */
>> + }
>> +
>> + return val;
>> +}
>> +
>> +static void krait_write_pmresrn(int n, u32 val)
>> +{
>> + switch (n) {
>> + case 0:
>> + asm volatile("mcr p15, 1, %0, c9, c15, 0" : : "r" (val));
>> + break;
>> + case 1:
>> + asm volatile("mcr p15, 1, %0, c9, c15, 1" : : "r" (val));
>> + break;
>> + case 2:
>> + asm volatile("mcr p15, 1, %0, c9, c15, 2" : : "r" (val));
>> + break;
>> + default:
>> + BUG(); /* Should be validated in krait_pmu_get_event_idx() */
>> + }
>> +}
> Do you need isbs to ensure the pmresrn side-effects have happened, or are
> the registers self-synchronising? Similarly for your other IMP DEF
> registers.
There aren't any isbs in the downstream android sources so I assume
they're self synchronizing. I'll confirm with the CPU designers to make
sure.
>
>> +static void krait_pre_vpmresr0(u32 *venum_orig_val, u32 *fp_orig_val)
>> +{
>> + u32 venum_new_val;
>> + u32 fp_new_val;
>> +
>> + /* CPACR Enable CP10 and CP11 access */
>> + *venum_orig_val = get_copro_access();
>> + venum_new_val = *venum_orig_val | CPACC_SVC(10) | CPACC_SVC(11);
>> + set_copro_access(venum_new_val);
>> +
>> + /* Enable FPEXC */
>> + *fp_orig_val = fmrx(FPEXC);
>> + fp_new_val = *fp_orig_val | FPEXC_EN;
>> + fmxr(FPEXC, fp_new_val);
> Messing around with the lot (especially with kernel-mode neon now in
> mainline) does scare me. I'd like some BUG_ON(preemptible()) and you could
> consider using kernel_neon_{begin,end} but they're a lot heavier than you
> need (due to non-lazy switching)
>
> Finally, I'd really like to see this get some test coverage, but I don't
> want to try running mainline on my phone :) Could you give your patches a
> spin with Vince's perf fuzzer please?
>
> https://github.com/deater/perf_event_tests.git
>
> (then build the contents of the fuzzer directory and run it for as long as
> you can).
>
Ok. I'll see what I can do.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Tomasz Figa @ 2014-01-21 18:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390190215-22700-2-git-send-email-olof@lixom.net>
Hi,
On 20.01.2014 04:56, Olof Johansson wrote:
> This patch enables support for power-on sequencing of SDIO peripherals through DT.
>
> In general, it's quite common that wifi modules and other similar
> peripherals have several signals in addition to the SDIO interface that
> needs wiggling before the module will power on. It's common to have a
> reference clock, one or several power rails and one or several lines
> for reset/enable type functions.
>
> The binding as written today introduces a number of reset gpios,
> a regulator and a clock specifier. The code will handle up to 2 gpio
> reset lines, but it's trivial to increase to more than that if needed
> at some point.
>
> Implementation-wise, the MMC core has been changed to handle this during
> host power up, before the host interface is powered on. I have not yet
> implemented the power-down side, I wanted people to have a chance for
> reporting back w.r.t. issues (or comments on the bindings) first.
>
> I have not tested the regulator portion, since the system and module
> I'm working on doesn't need one (Samsung Chromebook with Marvell
> 8797-based wifi). Testing of those portions (and reporting back) would
> be appreciated.
While I fully agree that this is an important problem that needs to be
solved, I really don't think this is the right way, because:
a) power-up sequence is really specific to the MMC device and often it's
not simply a matter of switching on one regulator or one clock, e.g.
specific time constraints need to be met.
b) you can have WLAN chips in which SDIO is just one of the options to
use as host interface, which may be also HSIC, I2C or UART. Really. See [1].
c) this is leaking device specific details to generic host code, which
isn't really elegant.
Now, to make this a bit more constructive, [2] is a solution that I came
up with (not perfect either), which simply adds a separate platform
device for the low level part of the chip. I believe this is a better
solution because:
a) you can often see such WLAN/BT combo chip as a set of separate
devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
which provides power/reset control, out of band signalling and etc. for
the first two, so it isn't that bad to have a separate device node for
the last one,
b) you have full freedom of defining your DT binding with whatever data
you need, any number of clocks, regulators, GPIOs and even out of band
interrupts (IMHO the most important one).
c) you can implement power-on, power-off sequences as needed for your
particular device,
d) you have full separation of device-specific data from MMC core (or
any other subsystem simply used as a way to perform I/O to the chip).
Now what's missing there is a way to signal the MMC core or any other
transport that a device showed up and the controller should be woken up
out of standby and scan of the bus initialized. This could be done by
explicitly specifying the device as a subnode of the
MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
controller of the chip or the other way around - a link to the
MMC/UART/... controller from the power controller node.
What do you think?
Best regards,
Tomasz
References:
[1] - http://www.marvell.com/wireless/assets/marvell_avastar_88w8797.pdf
[2] -
https://review.tizen.org/git/?p=platform/kernel/linux-3.10.git;a=commitdiff;h=978bc9523622248271e4007330ae1a0eee6e0254
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
> Documentation/devicetree/bindings/mmc/mmc.txt | 11 +++++++
> drivers/mmc/core/core.c | 42 +++++++++++++++++++++++++
> drivers/mmc/core/host.c | 30 +++++++++++++++++-
> include/linux/mmc/host.h | 5 +++
> 4 files changed, 87 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 458b57f..962e0ee 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -5,6 +5,8 @@ these definitions.
> Interpreted by the OF core:
> - reg: Registers location and length.
> - interrupts: Interrupts used by the MMC controller.
> +- clocks: Clocks needed for the host controller, if any.
> +- clock-names: Goes with clocks above.
>
> Card detection:
> If no property below is supplied, host native card detect is used.
> @@ -30,6 +32,15 @@ Optional properties:
> - cap-sdio-irq: enable SDIO IRQ signalling on this interface
> - full-pwr-cycle: full power cycle of the card is supported
>
> +Card power and reset control:
> +The following properties can be specified for cases where the MMC
> +peripheral needs additional reset, regulator and clock lines. It is for
> +example common for WiFi/BT adapters to have these separate from the main
> +MMC bus:
> + - card-reset-gpios: Specify GPIOs for card reset (reset active low)
> + - card-external-vcc-supply: Regulator to drive (independent) card VCC
> + - clock with name "card_ext_clock": External clock provided to the card
> +
> *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
> polarity properties, we have to fix the meaning of the "normal" and "inverted"
> line levels. We choose to follow the SDHCI standard, which specifies both those
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 098374b..c43e6c8 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -13,11 +13,13 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> +#include <linux/clk.h>
> #include <linux/completion.h>
> #include <linux/device.h>
> #include <linux/delay.h>
> #include <linux/pagemap.h>
> #include <linux/err.h>
> +#include <linux/gpio.h>
> #include <linux/leds.h>
> #include <linux/scatterlist.h>
> #include <linux/log2.h>
> @@ -1519,6 +1521,43 @@ void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type)
> mmc_host_clk_release(host);
> }
>
> +static void mmc_card_power_up(struct mmc_host *host)
> +{
> + int i;
> + struct gpio_desc **gds = host->card_reset_gpios;
> +
> + for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
> + if (gds[i]) {
> + dev_dbg(host->parent, "Asserting reset line %d", i);
> + gpiod_set_value(gds[i], 1);
> + }
> + }
> +
> + if (host->card_regulator) {
> + dev_dbg(host->parent, "Enabling external regulator");
> + if (regulator_enable(host->card_regulator))
> + dev_err(host->parent, "Failed to enable external regulator");
> + }
> +
> + if (host->card_clk) {
> + dev_dbg(host->parent, "Enabling external clock");
> + clk_prepare_enable(host->card_clk);
> + }
> +
> + /* 2ms delay to let clocks and power settle */
> + mmc_delay(20);
> +
> + for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
> + if (gds[i]) {
> + dev_dbg(host->parent, "Deasserting reset line %d", i);
> + gpiod_set_value(gds[i], 0);
> + }
> + }
> +
> + /* 2ms delay to after reset release */
> + mmc_delay(20);
> +}
> +
> /*
> * Apply power to the MMC stack. This is a two-stage process.
> * First, we enable power to the card without the clock running.
> @@ -1535,6 +1574,9 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
> if (host->ios.power_mode == MMC_POWER_ON)
> return;
>
> + /* Power up the card/module first, if needed */
> + mmc_card_power_up(host);
> +
> mmc_host_clk_hold(host);
>
> host->ios.vdd = fls(ocr) - 1;
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 49bc403..e6b850b 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -12,14 +12,18 @@
> * MMC host class device management
> */
>
> +#include <linux/kernel.h>
> +#include <linux/clk.h>
> #include <linux/device.h>
> #include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/idr.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> #include <linux/pagemap.h>
> #include <linux/export.h>
> #include <linux/leds.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> #include <linux/suspend.h>
>
> @@ -312,7 +316,7 @@ int mmc_of_parse(struct mmc_host *host)
> u32 bus_width;
> bool explicit_inv_wp, gpio_inv_wp = false;
> enum of_gpio_flags flags;
> - int len, ret, gpio;
> + int i, len, ret, gpio;
>
> if (!host->parent || !host->parent->of_node)
> return 0;
> @@ -415,6 +419,30 @@ int mmc_of_parse(struct mmc_host *host)
> if (explicit_inv_wp ^ gpio_inv_wp)
> host->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>
> + /* Parse card power/reset/clock control */
> + if (of_find_property(np, "card-reset-gpios", NULL)) {
> + struct gpio_desc *gpd;
> + for (i = 0; i < ARRAY_SIZE(host->card_reset_gpios); i++) {
> + gpd = devm_gpiod_get_index(host->parent, "card-reset", i);
> + if (IS_ERR(gpd))
> + break;
> + gpiod_direction_output(gpd, 0);
> + host->card_reset_gpios[i] = gpd;
> + }
> +
> + gpd = devm_gpiod_get_index(host->parent, "card-reset", ARRAY_SIZE(host->card_reset_gpios));
> + if (!IS_ERR(gpd)) {
> + dev_warn(host->parent, "More reset gpios than we can handle");
> + gpiod_put(gpd);
> + }
> + }
> +
> + host->card_clk = of_clk_get_by_name(np, "card_ext_clock");
> + if (IS_ERR(host->card_clk))
> + host->card_clk = NULL;
> +
> + host->card_regulator = regulator_get(host->parent, "card-external-vcc");
> +
> if (of_find_property(np, "cap-sd-highspeed", &len))
> host->caps |= MMC_CAP_SD_HIGHSPEED;
> if (of_find_property(np, "cap-mmc-highspeed", &len))
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 99f5709..6781887 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -297,6 +297,11 @@ struct mmc_host {
> unsigned long clkgate_delay;
> #endif
>
> + /* card specific properties to deal with power and reset */
> + struct regulator *card_regulator; /* External VCC needed by the card */
> + struct gpio_desc *card_reset_gpios[2]; /* External resets, active low */
> + struct clk *card_clk; /* External clock needed by the card */
> +
> /* host specific block data */
> unsigned int max_seg_size; /* see blk_queue_max_segment_size */
> unsigned short max_segs; /* see blk_queue_max_segments */
>
^ permalink raw reply
* [PATCH v3 16/24] drm/i2c: tda998x: add DT documentation
From: Olof Johansson @ 2014-01-21 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140120105410.15705f32@armhf>
On Mon, Jan 20, 2014 at 1:54 AM, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Sun, 19 Jan 2014 20:06:09 -0800
> Olof Johansson <olof@lixom.net> wrote:
>
>> Hi,
>>
>> On Sun, Jan 19, 2014 at 10:58 AM, Jean-Francois Moine <moinejf@free.fr> wrote:
>> > Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
>> > ---
>> > .../devicetree/bindings/drm/i2c/tda998x.txt | 24 ++++++++++++++++++++++
>> > 1 file changed, 24 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>>
>> Please cc bindings for review to devicetree at vger.kernel.org (cc:d here now)
>>
>> > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > new file mode 100644
>> > index 0000000..72da71d
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > @@ -0,0 +1,24 @@
>> > +Device-Tree bindings for the NXP TDA998x HDMI transmitter
>> > +
>> > +Required properties;
>> > + - compatible: must be "nxp,tda998x"
>> > +
>> > +Optional properties:
>> > + - interrupts: interrupt number for HDMI exchanges - default: by polling
>>
>> What are HDMI exchanges, and how do they differ from other interrupts?
>
> The used HDMI interrupt events are screen plug/unplug and EDID read.
> There are also CEC read/write which are not yet implemented in the
> tda998x driver.
>
> There is no difference from normal interrupts, except that the events
> may be get by polling, so, the interrupt number is optional.]
Ok, then it looks like the property description is a little confusing.
I'd remove the mentioning of HDMI exchanges from it.
>> > +
>> > + - pinctrl-0: pin control group to be used for this controller (IRQ).
>> > +
>> > + - pinctrl-names: must contain a "default" entry.
>> > +
>> > + - video-ports: 24 bits value - default: <0x230145>
>>
>> What is this?
>
> The video-ports value defines how the video controller is connected to
> the tda998x chip. Each 4 bits value tells from which input pins comes
> the video data and if there is any bit inversion. Each byte of this
> video-ports is used to load the VIP_CNTRL_{0,1,2} registers. These ones
> are described in the TDA9983B documentation which is the closer
> available document about the TDA998x family.
>
> The default value is the one defined for TI boards.
> A known other value is <0x234501> which is used for Russell's Armada
> DRM driver in the Cubox (Marvell A510), but this driver has no DT
> support.
Ok, this is a classic case where the binding should describe how
things are configured/wired up instead of hardcoding a register value
like this. From the data sheet there seems to be a _lot_ of settings,
so selecting the needed subset for now might be acceptable (and go
with defaults on the rest).
-Olof
^ permalink raw reply
* [PATCH v4] arm: remove !CPU_V6 and !GENERIC_ATOMIC64 build dependencies for XEN
From: Stefano Stabellini @ 2014-01-21 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121180750.GO30706@mudshark.cambridge.arm.com>
On Tue, 21 Jan 2014, Will Deacon wrote:
> On Tue, Jan 21, 2014 at 01:44:24PM +0000, Stefano Stabellini wrote:
> > Remove !GENERIC_ATOMIC64 build dependency:
> > - introduce xen_atomic64_xchg
> > - use it to implement xchg_xen_ulong
> >
> > Remove !CPU_V6 build dependency:
> > - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> > CONFIG_CPU_V6
> > - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > CC: arnd at arndb.de
> > CC: linux at arm.linux.org.uk
> > CC: will.deacon at arm.com
> > CC: catalin.marinas at arm.com
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: linux-kernel at vger.kernel.org
> > CC: xen-devel at lists.xenproject.org
>
> Reviewed-by: Will Deacon <will.deacon@arm.com>
>
> Cheers Stefano,
Thanks!!
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Olof Johansson @ 2014-01-21 18:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFpfrs=e3bDvdFRpnD3RkmD8ZvAJgFi6XvCRAOocqAKkKw@mail.gmail.com>
On Tue, Jan 21, 2014 at 12:55 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
[pruning quotes a bit]
>>> Is the above regulator related to host->ocr_avail mask? Could the
>>> above regulator be replaced by vmmc?
>>
>> I have to admit that I don't know MMC as well as I could, but OCR seems to be
>> something that's between the driver/controller/device, not related to external
>> power control in this case?
>
> This is related to the power of the card, typically external
> regulators controlled by the host driver.
>
> Both the card and the host supports a voltage range. This range is
> exactly what the OCR mask describes. At initialization of the card,
> the host ocr is validated against the card ocr.
Ok, so they specify the voltage needed, but there's no way to
determine what regulator is wired up to control. So that information
is still needed (possibly through vmmc though).
>>> At the moment host drivers uses mmc_regulator_get_supply(), which
>>> fetches regulators called "vmmc" and "vqmmc". It is also common to
>>> have these defined in DT like "vmmc-supply". This has not been
>>> properly documented for most host cases, and we should fix that. I
>>> also think it would make sense to include these in the documentation
>>> for the common mmc bindings, instead of host specific bindings.
>>
>> Hm, I had been of the impression that the vmmc stuff is to control
>> power/voltage on the signal lines, not for external card power. Still, even in
>> that case there's need for the reset line handling and clock control.
>
> vmmc: the power to the card.
> vqmmc: the I/O voltage levels (for the signal lines).
Ah, ok. So vmmc seems like it's the same then. I'll try to get some
cycles to look at it today.
> Regarding reset, I agree, those seems to be needed.
>
> Regarding clock control. I suppose you are referring to separate
> external clocks, not affecting the SDIO/SD/MMC interface speed!?
>
> That could make sense, but still I wonder how those shall be handled
> in a fine grained power management setup. In other words, when shall
> those be gated/ungated? Is the mmc core able to take the correct
> decision about these?
The reference clock is in most cases I've seen 32kHz, and not
something that's under fine-grained power management. So it's not used
to regulate interface speed, etc.
-Olof
^ permalink raw reply
* Question: about wm8904's regulator consumer nodes
From: Mark Brown @ 2014-01-21 18:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <B256D81BAE5131468A838E5D7A24364172F64369@penmbx01>
On Tue, Jan 21, 2014 at 07:10:24AM +0000, Yang, Wenyou wrote:
> DCVDD-supply = <&vcc_1v8_reg>;
> DBVDD-supply = <&vddana_reg>;
> AVDD-supply = <&vcc_1v8_reg>;
> CPVDD-supply = <&vcc_1v8_reg>;
> MICVDD-supply = <&vddana_reg>;
> But the power of DCVDD, DBVDD, AVDD, CPVDD and MICVDD is not supplied by any regulators, not the above regulator nodes.
> How do I deal with this case? Could you give me some advice?
You should set up the supplies to point to whatever regulator is
supplying the power. If there are supplies on the board which are
generated using fixed voltage regulators then you should register those
using the fixed regulator driver. Recent kernels are more tolerant of
missing supplies but it's still better to be explicit if you can be.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140121/c4ed9767/attachment.sig>
^ permalink raw reply
* [PATCH 1/2] ohci-platform: Add support for controllers with big-endian regs / descriptors
From: Alan Stern @ 2014-01-21 18:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DEA7CD.5070107@redhat.com>
On Tue, 21 Jan 2014, Hans de Goede wrote:
> Hi,
>
> On 01/21/2014 05:40 PM, Alan Stern wrote:
> > On Tue, 21 Jan 2014, Hans de Goede wrote:
> >
> >> Note this commit uses the same devicetree booleans for this as the ones
> >> already existing in the usb-ehci bindings.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> >> --- a/drivers/usb/host/Kconfig
> >> +++ b/drivers/usb/host/Kconfig
> >> @@ -512,6 +512,10 @@ config USB_CNS3XXX_OHCI
> >>
> >> config USB_OHCI_HCD_PLATFORM
> >> tristate "Generic OHCI driver for a platform device"
> >> + # Always support LE, support BE on architectures which have readl_be
> >> + select USB_OHCI_LITTLE_ENDIAN
> >> + select USB_OHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
> >> + select USB_OHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
> >> default n
> > In any case, the style we have adopted is that these select lines go in
> > the arch-specific defconfig, not here.
>
> Ok, so I should drop the Kconfig parts of both patches ?
That's rigt. They are likely to cause trouble, and if the selects were
needed then they should already be present somewhere else.
Alan Stern
^ permalink raw reply
* [PATCH v4] arm: remove !CPU_V6 and !GENERIC_ATOMIC64 build dependencies for XEN
From: Will Deacon @ 2014-01-21 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390311864-19119-1-git-send-email-stefano.stabellini@eu.citrix.com>
On Tue, Jan 21, 2014 at 01:44:24PM +0000, Stefano Stabellini wrote:
> Remove !GENERIC_ATOMIC64 build dependency:
> - introduce xen_atomic64_xchg
> - use it to implement xchg_xen_ulong
>
> Remove !CPU_V6 build dependency:
> - introduce __cmpxchg8 and __cmpxchg16, compiled even ifdef
> CONFIG_CPU_V6
> - implement sync_cmpxchg using __cmpxchg8 and __cmpxchg16
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: arnd at arndb.de
> CC: linux at arm.linux.org.uk
> CC: will.deacon at arm.com
> CC: catalin.marinas at arm.com
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
> CC: xen-devel at lists.xenproject.org
Reviewed-by: Will Deacon <will.deacon@arm.com>
Cheers Stefano,
Will
^ permalink raw reply
* [PATCH v2 5/7] ARM: perf_event: Fully support Krait CPU PMU events
From: Will Deacon @ 2014-01-21 18:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389808535-23852-6-git-send-email-sboyd@codeaurora.org>
Hi Stephen,
Thanks for the updates. A few more comments inline.
On Wed, Jan 15, 2014 at 05:55:33PM +0000, Stephen Boyd wrote:
> Krait supports a set of performance monitor region event
> selection registers (PMRESR) sitting behind a cp15 based
> interface that extend the architected PMU events to include Krait
> CPU and Venum VFP specific events. To use these events the user
> is expected to program the region register (PMRESRn) with the
> event code shifted into the group they care about and then point
> the PMNx event at that region+group combo by writing a
> PMRESRn_GROUPx event. Add support for this hardware.
>
> Note: the raw event number is a pure software construct that
> allows us to map the multi-dimensional number space of regions,
> groups, and event codes into a flat event number space suitable
> for use by the perf framework.
[...]
> +static u32 krait_read_pmresrn(int n)
> +{
> + u32 val;
> +
> + switch (n) {
> + case 0:
> + asm volatile("mrc p15, 1, %0, c9, c15, 0" : "=r" (val));
> + break;
> + case 1:
> + asm volatile("mrc p15, 1, %0, c9, c15, 1" : "=r" (val));
> + break;
> + case 2:
> + asm volatile("mrc p15, 1, %0, c9, c15, 2" : "=r" (val));
> + break;
> + default:
> + BUG(); /* Should be validated in krait_pmu_get_event_idx() */
> + }
> +
> + return val;
> +}
> +
> +static void krait_write_pmresrn(int n, u32 val)
> +{
> + switch (n) {
> + case 0:
> + asm volatile("mcr p15, 1, %0, c9, c15, 0" : : "r" (val));
> + break;
> + case 1:
> + asm volatile("mcr p15, 1, %0, c9, c15, 1" : : "r" (val));
> + break;
> + case 2:
> + asm volatile("mcr p15, 1, %0, c9, c15, 2" : : "r" (val));
> + break;
> + default:
> + BUG(); /* Should be validated in krait_pmu_get_event_idx() */
> + }
> +}
Do you need isbs to ensure the pmresrn side-effects have happened, or are
the registers self-synchronising? Similarly for your other IMP DEF
registers.
> +static void krait_pre_vpmresr0(u32 *venum_orig_val, u32 *fp_orig_val)
> +{
> + u32 venum_new_val;
> + u32 fp_new_val;
> +
> + /* CPACR Enable CP10 and CP11 access */
> + *venum_orig_val = get_copro_access();
> + venum_new_val = *venum_orig_val | CPACC_SVC(10) | CPACC_SVC(11);
> + set_copro_access(venum_new_val);
> +
> + /* Enable FPEXC */
> + *fp_orig_val = fmrx(FPEXC);
> + fp_new_val = *fp_orig_val | FPEXC_EN;
> + fmxr(FPEXC, fp_new_val);
Messing around with the lot (especially with kernel-mode neon now in
mainline) does scare me. I'd like some BUG_ON(preemptible()) and you could
consider using kernel_neon_{begin,end} but they're a lot heavier than you
need (due to non-lazy switching)
Finally, I'd really like to see this get some test coverage, but I don't
want to try running mainline on my phone :) Could you give your patches a
spin with Vince's perf fuzzer please?
https://github.com/deater/perf_event_tests.git
(then build the contents of the fuzzer directory and run it for as long as
you can).
Cheers,
Will
^ permalink raw reply
* [PATCH] mmc: omap_hsmmc: Add support for Erratum 2.1.1.128 in device tree boot
From: Felipe Balbi @ 2014-01-21 18:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DEB078.5030005@ti.com>
Hi,
On Tue, Jan 21, 2014 at 11:38:00AM -0600, Nishanth Menon wrote:
> On 01/20/2014 05:39 PM, Felipe Balbi wrote:
> > On Mon, Jan 20, 2014 at 05:29:02PM -0600, Nishanth Menon wrote:
> >> When device is booted using devicetree, platforms impacted by
> >> Erratum 2.1.1.128 is not detected easily in the mmc driver. This erratum
> >> indicates that the module cannot do multi-block transfers.
> >>
> >> Handle this by providing a boolean flag to indicate to driver that it is
> >> working on a hardware with mentioned limitation.
> >
> > sure there's no way of reading the revision register to figure this one
> > out without having to add a new DT attribute ?
> >
> I did a quick patch to read the Module revision register:
> http://slexy.org/view/s21TKvlWlR
>
> sdp2430: Revision: 1.2, Spec: 0.0, normal interrupt
>
> OMAP3430-ldp: (ES2.1): Revision: 2.6, Spec: 0.0, normal interrupt
> SDP3430:(ES3.0) Revision: 2.6, Spec: 0.0, normal interrupt
> AM3517-evm: (ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
> AM3517-crane:(ES1.1): Revision: 2.6, Spec: 0.0, normal interrupt
>
> AM37x-evm: (ES1.2) Revision: 2.6, Spec: 0.0, normal interrupt
> OMAP3630-beag-xm (ES1.2): Revision: 2.6, Spec: 0.0, normal interrupt
>
> am335x-evm:(ES1.0): Revision: 3.1, Spec: 0.1, normal interrupt
> am335x-sk: (ES2.1): Revision: 3.1, Spec: 0.1, normal interrupt
> am335x-beaglebone-black:(ES2.0): Revision: 3.1, Spec: 0.1, normal
> interrupt
>
> sdp4430.txt: (ES2.2): Revision: 3.1, Spec: 0.1, normal interrupt
>
> OMAP4460-panda-es (ES1.1): Revision: 3.1, Spec: 0.1, normal interrupt
>
> OMAP5uevm:(ES2.0): Revision: 3.3, Spec: 0.2, normal interrupt
> dra7-evm (es1.1): Revision: 3.3, Spec: 0.2, normal interrupt
>
>
> OMAP3430-ldp seems to be the only one impacted with module revision
> 2.6 -> so using revision information is not really helpful here. Hence
> the usage of a flag in dt attribute to indicate hardware impacted by
> erratum.
alright, that's too bad. Seems like revision in this module isn't very
useful :-(
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140121/fde09f0c/attachment.sig>
^ permalink raw reply
* [PATCH v7 1/2] ohci-platform: Add support for devicetree instantiation
From: Sergei Shtylyov @ 2014-01-21 17:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389813883-567-2-git-send-email-hdegoede@redhat.com>
Hello.
On 01/15/2014 10:24 PM, Hans de Goede wrote:
> Add support for ohci-platform instantiation from devicetree, including
> optionally getting clks and a phy from devicetree, and enabling / disabling
> those on power_on / off.
> This should allow using ohci-platform from devicetree in various cases.
> Specifically after this commit it can be used for the ohci controller found
> on Allwinner sunxi SoCs.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
[...]
Have only found time to fully read the patches just now...
> diff --git a/Documentation/devicetree/bindings/usb/usb-ohci.txt b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> new file mode 100644
> index 0000000..f9d6c73
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-ohci.txt
> @@ -0,0 +1,22 @@
> +USB OHCI controllers
> +
> +Required properties:
> +- compatible : "usb-ohci"
> +- reg : ohci controller register range (address and length)
> +- interrupts : ohci controller interrupt
> +
> +Optional properties:
> +- clocks : a list of phandle + clock specifier pairs
> +- phys : phandle + phy specifier pair
> +- phy-names : "usb"
> +
> +Example:
> +
> + ohci0: ohci at 0x01c14400 {
Two minor nits: there should be no "0x" in the address part of the node
name. And according to ePAPR [1], "the name of a node should be somewhat
generic, reflecting the function of the device and not its precise programming
model. If appropriate, the name should be one of the following choices:
[...]
- usb".
Same comments for "usb-ehci" binding.
> + compatible = "allwinner,sun4i-a10-ohci", "usb-ohci";
> + reg = <0x01c14400 0x100>;
> + interrupts = <64>;
> + clocks = <&usb_clk 6>, <&ahb_gates 2>;
> + phys = <&usbphy 1>;
> + phy-names = "usb";
> + };
[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
WBR, Sergei
^ permalink raw reply
* [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block
From: Varun Sethi @ 2014-01-21 17:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140120222814.GI3471@alberich>
> -----Original Message-----
> From: Andreas Herrmann [mailto:andreas.herrmann at calxeda.com]
> Sent: Tuesday, January 21, 2014 3:58 AM
> To: Will Deacon
> Cc: iommu at lists.linux-foundation.org; linux-arm-
> kernel at lists.infradead.org; Andreas Herrmann; Sethi Varun-B16395
> Subject: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier
> block
>
>
> At the moment just handle IOMMU_GROUP_NOTIFY_BIND_DRIVER to conditionally
> isolate all master devices for an SMMU.
>
> Depending on DT information each device is put into its own protection
> domain (if possible). For configuration with one or just a few masters
> per SMMU that is easy to achieve.
>
> In case of many devices per SMMU (e.g. MMU-500 with it's distributed
> translation support) isolation of each device might not be possible --
> depending on number of available SMR groups and/or context banks.
>
> Default is that device isolation is contolled per SMMU with SMMU node
> property "arm,smmu-isolate-devices" in a DT. If this property is set for
> an SMMU node, device isolation is performed.
>
> W/o device isolation the driver detects SMMUs but no translation is
> configured (transactions just bypass translation process).
>
> Note that for device isolation dma_base and size are fixed as 0 and
> SZ_128M at the moment. Additional patches will address this restriction
> and allow automatic growth of mapping size.
>
> Cc: Varun Sethi <Varun.Sethi@freescale.com>
> Cc: Andreas Herrmann <herrmann.der.user@googlemail.com>
> Signed-off-by: Andreas Herrmann <andreas.herrmann@calxeda.com>
> ---
> drivers/iommu/arm-smmu.c | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> Hi Will,
>
> This new patch addresses Varun's comments:
> - use iommu_group notifier instead of bus notifier
> - remove superfluous call to arm_smmu_add_device in
> notifier function
>
> This patch depends on commit "iommu/arm-smmu: add devices attached to the
> SMMU to an IOMMU group" as found in your git tree (e.g. in branch
> iommu/devel or for-joerg/arm-smmu/updates).
>
>
> Andreas
>
> PS: This time with a proper adaption of the notifier function.
>
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index
> 0a5649f..da19bd6 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -46,6 +46,7 @@
> #include <linux/amba/bus.h>
>
> #include <asm/pgalloc.h>
> +#include <asm/dma-iommu.h>
>
> /* Driver options */
> #define ARM_SMMU_OPT_ISOLATE_DEVICES (1 << 0)
> @@ -1517,6 +1518,47 @@ static int arm_smmu_domain_has_cap(struct
> iommu_domain *domain,
> return !!(cap & caps);
> }
>
> +static int arm_smmu_group_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct device *dev = data;
> + struct dma_iommu_mapping *mapping;
> + struct arm_smmu_device *smmu;
> + int ret;
> +
> + switch (action) {
> + case IOMMU_GROUP_NOTIFY_BIND_DRIVER:
> +
> + smmu = dev->archdata.iommu;
> + if (!smmu || !(smmu->options & ARM_SMMU_OPT_ISOLATE_DEVICES))
> + break;
[Sethi Varun-B16395] Should this check be really done here? The "Isolate devices" property would allow us to set up iommu groups. My understanding is that if we specify the isolate devices property, then each device would have a separate iommu group otherwise all devices connected to the SMMU would share the iommu group.
With that logic, we should link the mapping to the iommu group.
-Varun
> +
> + mapping = arm_iommu_create_mapping(&platform_bus_type,
> + 0, SZ_128M, 0);
> + if (IS_ERR(mapping)) {
> + ret = PTR_ERR(mapping);
> + dev_info(dev, "arm_iommu_create_mapping failed\n");
> + break;
> + }
> +
> + ret = arm_iommu_attach_device(dev, mapping);
> + if (ret < 0) {
> + dev_info(dev, "arm_iommu_attach_device failed\n");
> + arm_iommu_release_mapping(mapping);
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block group_nb = {
> + .notifier_call = arm_smmu_group_notifier, };
> +
> static int arm_smmu_add_device(struct device *dev) {
> struct arm_smmu_device *child, *parent, *smmu; @@ -1566,6 +1608,8
> @@ static int arm_smmu_add_device(struct device *dev)
> return PTR_ERR(group);
> }
>
> + iommu_group_register_notifier(group, &group_nb);
> +
> ret = iommu_group_add_device(group, dev);
> iommu_group_put(group);
> dev->archdata.iommu = smmu;
> --
> 1.7.9.5
>
>
^ 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