[PATCHv2 0/2]

[PATCHv2 0/2]

[Keith Busch]

NVMe devices had been able to continue sending interrupt messages while
the driver is handling the previous notification when threaded interrupts
are used. This can cause unnecessary CPU cycles spent in hard irq context,
and potentially triggers spurious interrupt detection if the threaded
handler runs sufficiently long: returning IRQ_WAKE_THREAD from the
primrary handler doesn't observe any changes to irq desc->thread_handled.

Use the appropriate masking for the interrupt type based on the NVMe
specification recommendations (see NVMe 1.4 section 7.5.1.1 for more
information) so that the primary handler does not get unnecessarily
triggered.

v1 -> v2:

  Don't split the nvme callbacks for MSI and MSI-x. Just check the type
  in the callback when selecting the masking method, and combine this
  in a single patch.

  Drop the more experiemental patches. That series will be posted
  separately.

Keith Busch (2):
  PCI/MSI: Export __pci_msix_desc_mask_irq
  nvme/pci: Mask device interrupts for threaded handlers

 drivers/nvme/host/pci.c | 28 ++++++++++++++++++++++++----
 drivers/pci/msi.c       |  1 +
 2 files changed, 25 insertions(+), 4 deletions(-)

-- 
2.21.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCHv2 1/2] PCI/MSI: Export __pci_msix_desc_mask_irq

[Keith Busch]

Export the fast msix mask for drivers to use when the read-back is
undesired.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/pci/msi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0884bedcfc7a..9e866929f4b0 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -225,6 +225,7 @@ u32 __pci_msix_desc_mask_irq(struct msi_desc *desc, u32 flag)
 
 	return mask_bits;
 }
+EXPORT_SYMBOL_GPL(__pci_msix_desc_mask_irq);
 
 static void msix_mask_irq(struct msi_desc *desc, u32 flag)
 {
-- 
2.21.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 1/2] PCI/MSI: Export __pci_msix_desc_mask_irq

[Christoph Hellwig]

On Tue, Dec 03, 2019 at 07:20:57AM +0900, Keith Busch wrote:
> Export the fast msix mask for drivers to use when the read-back is
> undesired.

As said last time calling this seems wrong as it breaks the irq_chip
abstraction.  But looking at the disable_irq_nosync semantics I think
that function should do a non-pasted disable disable for MSI(-X)
interrupts.  Can you look into that?

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 1/2] PCI/MSI: Export __pci_msix_desc_mask_irq

[Sebastian Andrzej Siewior]

On 2019-12-02 23:46:03 [+0100], Christoph Hellwig wrote:
> On Tue, Dec 03, 2019 at 07:20:57AM +0900, Keith Busch wrote:
> > Export the fast msix mask for drivers to use when the read-back is
> > undesired.
> 
> As said last time calling this seems wrong as it breaks the irq_chip
> abstraction.  But looking at the disable_irq_nosync semantics I think
> that function should do a non-pasted disable disable for MSI(-X)
> interrupts.  Can you look into that?

Using disable_irq_nosync() would be the same as using IRQF_ONESHOT which
is the preferred way.
Keith complained about this as slow and avoiding the read-back as
noticeable.

The generic way would be pci_msi_mask_irq() and the difference

|                 msix_mask_irq(desc, flag);
|                 readl(desc->mask_base);         /* Flush write to device */

would be that flush.

Sebastian

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 1/2] PCI/MSI: Export __pci_msix_desc_mask_irq

[Keith Busch]

On Tue, Dec 03, 2019 at 10:04:54AM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-12-02 23:46:03 [+0100], Christoph Hellwig wrote:
> > On Tue, Dec 03, 2019 at 07:20:57AM +0900, Keith Busch wrote:
> > > Export the fast msix mask for drivers to use when the read-back is
> > > undesired.
> > 
> > As said last time calling this seems wrong as it breaks the irq_chip
> > abstraction.  But looking at the disable_irq_nosync semantics I think
> > that function should do a non-pasted disable disable for MSI(-X)
> > interrupts.  Can you look into that?
> 
> Using disable_irq_nosync() would be the same as using IRQF_ONESHOT which
> is the preferred way.
> Keith complained about this as slow and avoiding the read-back as
> noticeable.
> 
> The generic way would be pci_msi_mask_irq() and the difference
> 
> |                 msix_mask_irq(desc, flag);
> |                 readl(desc->mask_base);         /* Flush write to device */
> 
> would be that flush.

Right, so the solution should be simply to remove the readl(). I'm pretty
sure that's safe to do: if mask_irq() returns before the device happens
to see the mask is set and generates an undesired interrupt, the irq
flow handler will observe irqd_irq_disabled() and return early. We have
to deal with that anyway because the device may have sent an interrupt
message at the same time the CPU was masking it. These would look
like the same thing from the CPU perspective.

I can't be completely sure it's safe for everyone though, so I'll try to
quantify the impact of the read back on nvme with some real hardware,
because I'm starting to wonder if this is really as important as I
initially thought.

If we do two readl()'s per IO, that's pretty noticable. It looks like
it's adding about 1usec to the completion latency (plus or minus, it
depends on the platform and if switches are involved).

But we certainly don't need this for each IO. For low depth workloads,
we can just handle all completions in the primary handler and never
mask interrupts.

In case there are lots of completions that are better handled in the
nvme_irq_thread(), then we can call disable_irq_nosync(). That doesn't
immediately mask MSIx because we didn't set IRQ_DISABLE_UNLAZY. The mask
won't happen until we see another interrupt, which means the thread
is going to be handling a lot of completions. The ratio of commands
processed to msix masking would be quite low. As far as I can tell,
the overhead seems pretty negligible.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCHv2 2/2] nvme/pci: Mask device interrupts for threaded handlers

[Keith Busch]

Local interrupts are reenabled when the nvme irq thread is woken, so
subsequent MSI or a level triggered interrupts may restart the nvme irq
check while the thread handler is running. This unnecessarily spends CPU
cycles and potentially triggers spurious interrupt detection, disabling
our nvme irq.

Prevent the controller from sending future messages. For legacy and MSI,
use the nvme interrupt mask/clear registers. For MSIx, use the quick
control mask function.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0590640ba62c..4022a872d29c 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -13,8 +13,10 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/irq.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/msi.h>
 #include <linux/mutex.h>
 #include <linux/once.h>
 #include <linux/pci.h>
@@ -1036,12 +1038,29 @@ static irqreturn_t nvme_irq(int irq, void *data)
 	return ret;
 }
 
+static irqreturn_t nvme_irq_thread(int irq, void *data)
+{
+	struct nvme_queue *nvmeq = data;
+
+	nvme_irq(irq, data);
+	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
+		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
+	else
+		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMC);
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t nvme_irq_check(int irq, void *data)
 {
 	struct nvme_queue *nvmeq = data;
-	if (nvme_cqe_pending(nvmeq))
-		return IRQ_WAKE_THREAD;
-	return IRQ_NONE;
+
+	if (!nvme_cqe_pending(nvmeq))
+		return IRQ_NONE;
+	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
+		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 1);
+	else
+		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMS);
+	return IRQ_WAKE_THREAD;
 }
 
 /*
@@ -1499,7 +1518,8 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
 
 	if (use_threaded_interrupts) {
 		return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq_check,
-				nvme_irq, nvmeq, "nvme%dq%d", nr, nvmeq->qid);
+				nvme_irq_thread, nvmeq, "nvme%dq%d", nr,
+				nvmeq->qid);
 	} else {
 		return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq,
 				NULL, nvmeq, "nvme%dq%d", nr, nvmeq->qid);
-- 
2.21.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 2/2] nvme/pci: Mask device interrupts for threaded handlers

[Christoph Hellwig]

On Tue, Dec 03, 2019 at 07:20:58AM +0900, Keith Busch wrote:
> +static irqreturn_t nvme_irq_thread(int irq, void *data)
> +{
> +	struct nvme_queue *nvmeq = data;
> +
> +	nvme_irq(irq, data);
> +	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
> +		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
> +	else
> +		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMC);

So independent of the indirection issue can we have a theory of operation
on why not using a read to flush the posted writes to disable/enable the
interrupt (either variant) here is fine?  Let's assume we have a worse
case implementation where no write ever gets delivered to the device
until we do a read neither of them will ever hit the device as we don't
really do MMIO reads in nvme during normal operation.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 2/2] nvme/pci: Mask device interrupts for threaded handlers

[Keith Busch]

On Tue, Dec 03, 2019 at 08:47:23AM +0100, Christoph Hellwig wrote:
> On Tue, Dec 03, 2019 at 07:20:58AM +0900, Keith Busch wrote:
> > +static irqreturn_t nvme_irq_thread(int irq, void *data)
> > +{
> > +	struct nvme_queue *nvmeq = data;
> > +
> > +	nvme_irq(irq, data);
> > +	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
> > +		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
> > +	else
> > +		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMC);
> 
> So independent of the indirection issue can we have a theory of operation
> on why not using a read to flush the posted writes to disable/enable the
> interrupt (either variant) here is fine?  Let's assume we have a worse
> case implementation where no write ever gets delivered to the device
> until we do a read neither of them will ever hit the device as we don't
> really do MMIO reads in nvme during normal operation.

Sure.

First, if the masking MMIO write is stalled, the read back doesn't
make it complete any faster. It just means the flush takes that
much longer.

Also note that the write can't be reordered with other writes to
that device. You'd have to use writel_relaxed() for that.

We notify the nvme controller of new commands using doorbell writes.
If all writes are stalled, then the device doesn't have new commands
to complete, so there won't be any interrupts for the driver to
handle.

If we want to craft a worst case scenario where the device was aware
of the maximum possible commands on a queue (1023 for this driver),
we could theoretically observe that many MSIs for exactly 1 IRQ_HANDLED
return. This is below the 0.01% threshold for "nobody cared".
Subsequent new commands would be stuck behind the MSI mask write,
so there can't possibly be new IO while that mask write is
stuck.

So, I think if the device never sees the MSI masking while the
thread is reaping the completion queue, we'll just spend CPU time
in an unnecessary irq handler that would have otherwise been consumed
on a flushing readl().

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 2/2] nvme/pci: Mask device interrupts for threaded handlers

[Sironi, Filippo]

> On 2. Dec 2019, at 23:20, Keith Busch <kbusch@kernel.org> wrote:
> 
> Local interrupts are reenabled when the nvme irq thread is woken, so
> subsequent MSI or a level triggered interrupts may restart the nvme irq
> check while the thread handler is running. This unnecessarily spends CPU
> cycles and potentially triggers spurious interrupt detection, disabling
> our nvme irq.
> 
> Prevent the controller from sending future messages. For legacy and MSI,
> use the nvme interrupt mask/clear registers. For MSIx, use the quick
> control mask function.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/nvme/host/pci.c | 28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 0590640ba62c..4022a872d29c 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -13,8 +13,10 @@
> #include <linux/init.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> +#include <linux/irq.h>
> #include <linux/mm.h>
> #include <linux/module.h>
> +#include <linux/msi.h>
> #include <linux/mutex.h>
> #include <linux/once.h>
> #include <linux/pci.h>
> @@ -1036,12 +1038,29 @@ static irqreturn_t nvme_irq(int irq, void *data)
> 	return ret;
> }
> 
> +static irqreturn_t nvme_irq_thread(int irq, void *data)
> +{
> +	struct nvme_queue *nvmeq = data;
> +
> +	nvme_irq(irq, data);
> +	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
> +		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 0);
> +	else
> +		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMC);
> +	return IRQ_HANDLED;
> +}
> +
> static irqreturn_t nvme_irq_check(int irq, void *data)
> {
> 	struct nvme_queue *nvmeq = data;
> -	if (nvme_cqe_pending(nvmeq))
> -		return IRQ_WAKE_THREAD;
> -	return IRQ_NONE;
> +
> +	if (!nvme_cqe_pending(nvmeq))
> +		return IRQ_NONE;
> +	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
> +		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 1);

Have you considered that __pci_msix_desc_mask_irq will cause
a trap from guest to hypervisor mode when running virtualized?

> +	else
> +		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMS);

What's stopping us from always using this method?

> +	return IRQ_WAKE_THREAD;
> }
> 
> /*
> @@ -1499,7 +1518,8 @@ static int queue_request_irq(struct nvme_queue *nvmeq)
> 
> 	if (use_threaded_interrupts) {
> 		return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq_check,
> -				nvme_irq, nvmeq, "nvme%dq%d", nr, nvmeq->qid);
> +				nvme_irq_thread, nvmeq, "nvme%dq%d", nr,
> +				nvmeq->qid);
> 	} else {
> 		return pci_request_irq(pdev, nvmeq->cq_vector, nvme_irq,
> 				NULL, nvmeq, "nvme%dq%d", nr, nvmeq->qid);
> -- 
> 2.21.0
> 
> 
> _______________________________________________
> linux-nvme mailing list
> linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879




_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCHv2 2/2] nvme/pci: Mask device interrupts for threaded handlers

[hch]

On Wed, Dec 04, 2019 at 10:10:05AM +0000, Sironi, Filippo wrote:
> > +	if (to_pci_dev(nvmeq->dev->dev)->msix_enabled)
> > +		__pci_msix_desc_mask_irq(irq_get_msi_desc(irq), 1);
> 
> Have you considered that __pci_msix_desc_mask_irq will cause
> a trap from guest to hypervisor mode when running virtualized?

As mentioned in my reply we need to route this through the irq chip
abstraction.  With that a PV irq chip handler doesn't need to trap if
it has smarter means available.

> > +	else
> > +		writel(1 << nvmeq->cq_vector, nvmeq->dev->bar + NVME_REG_INTMS);
> 
> What's stopping us from always using this method?

The fact that the NVMe spec doesn't specify it for MSI-X.  From section
3.1.3 of NVMe 1.4:

"This register is used to mask interrupts when using pin-based interrupts,
single message MSI, or multiple message MSI. When using MSI-X, the
interrupt mask table defined as part of MSI-X should be used to mask
interrupts. Host software shall not access this register when configured
for MSI-X; any accesses when configured for MSI-X is undefined. For
interrupt behavior requirements, refer to section 7.5."

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 2/2] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

[Kishon Vijay Abraham I]

Hi,

On Monday 18 September 2017 07:50 PM, Andrzej Pietrasiewicz wrote:
> Hi,
> 
> W dniu 18.09.2017 o 14:43, Felipe Balbi pisze:
>>
>> Hi,
>>
>> Andrzej Pietrasiewicz <andrzej.p-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> writes:
>>>>>>> +static int exynos5_usbdrd_phy_reset(struct phy *phy)
>>>>>>> +{
>>>>>>> +    struct phy_usb_instance *inst = phy_get_drvdata(phy);
>>>>>>> +    struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
>>>>>>> +
>>>>>>> +    return exynos5420_usbdrd_phy_calibrate(phy_drd);
>>>>>>> +}
>>>>>>> +
>>>>>>>     static const struct phy_ops exynos5_usbdrd_phy_ops = {
>>>>>>>         .init        = exynos5_usbdrd_phy_init,
>>>>>>>         .exit        = exynos5_usbdrd_phy_exit,
>>>>>>>         .power_on    = exynos5_usbdrd_phy_power_on,
>>>>>>>         .power_off    = exynos5_usbdrd_phy_power_off,
>>>>>>> +    .reset        = exynos5_usbdrd_phy_reset,
>>>>>>>         .owner        = THIS_MODULE,
>>>>>>>     };
>>>>>>>     diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>>>> index 03474d3..1d5836e 100644
>>>>>>> --- a/drivers/usb/dwc3/core.c
>>>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>>>> @@ -156,9 +156,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>>>>>>>             } else {
>>>>>>>                 if (dwc->usb2_phy)
>>>>>>>                     otg_set_vbus(dwc->usb2_phy->otg, true);
>>>>>>> -            if (dwc->usb2_generic_phy)
>>>>>>> +            if (dwc->usb2_generic_phy) {
>>>>>>>                     phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>>>>> -
>>>>>>> +                phy_reset(dwc->usb2_generic_phy);
>>>>>>
>>>>>> it doesn't look like this is the best place to reset the phy. Also,
>>>>>
>>>>> right, phy_reset is done during initialization before
>>>>> phy_power_on/phy_init or
>>>>> in error cases.
>>>>>
>>>>>> ->reset() doesn't seem to match correctly with a calibration. That seems
>>>>>> to be more fitting to a ->power_on() or ->init() implementation.
>>>>>
>>>>> yeah, the initial patch seems to calibrate in phy_init(). Not sure why it's
>>>>> modified.
>>>>
>>>> The original patch used a hack like below, in xhci_plat_probe():
>>>>
>>>> +       /* Initialize and power-on USB 3.0 PHY */
>>>> +       xhci->shared_hcd->phy->init_count = 0;
>>>> +       ret = phy_init(xhci->shared_hcd->phy);
>>>> +       if (ret)
>>>> +               goto dealloc_usb3_hcd;
>>>> +
>>>> +       xhci->shared_hcd->phy->power_count = 0;
>>>> +       ret = phy_power_on(xhci->shared_hcd->phy);
>>>> +       if (ret) {
>>>> +               phy_exit(xhci->shared_hcd->phy);
>>>> +               goto dealloc_usb3_hcd;
>>>> +       }
>>>> +
>>>>
>>>> Manually setting init_count to 0 in order for the subsequent phy_init() to
>>>> happen probably does not look good.
>>>>
>>>> The calibration is clearly needed. However, I don't have any strong opinions
>>>> on from which place exactly to trigger the calibration process.
>>>> The original patch did not make it upstream, but if that patch is ok,
>>>> it is perfectly fine with me to drop my version and take that one instead.
>>>
>>> Me bad, I did not write about an important issue.
>>> The calibration must happen after usb_add_hcd(), otherwise
>>> usb_add_hcd() indirectly triggers overwriting the effects of calibration.
>>
>> in that case, you should do that from xhci-plat indeed. I think the
>> whole idea with init_count is just to make sure you don't initialize it
>> twice.
> 
> As far as I understand the code in question the desired result is exactly the
> opposite:
> to make sure it _does_ initialize twice, otherwise after the first
> initialization the
> calibration results were lost. In other words, in the code snippet above,
> in xhci_plat_probe() the phy_init() was creatively (ab)used in order to force
> the calibration at a desired moment, while in the original invocation of
> phy_init()
> the calibration result was merely a short-term side effect discarded soon
> afterwards.
> 
>>
>> One thing's for sure, ->reset() doesn't seem to be the matching callback
>> for you to use and, given your explanation above, dwc3 doesn't seem to
>> be the right place to fiddle with that.
>>
>> Seems like we need an extension of the generic PHY framework to cope
>> with your requirement.
>>
> 
> Here are old patches from Vivek:
> 
> https://lkml.org/lkml/2014/9/2/166
> 
> In particular:
> 
> https://lkml.org/lkml/2014/9/2/170
> 
> Please see the discussion that follows the latter.
> 
> All in all, is adding the calibrate() method to phy_ops the way to go or not?

Adding calibrate is fine but doing init() and power_on() in one driver and
calibrate() in another doesn't look correct. Why not let xhci do init() and
power_on() of phy instead of dwc3?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/2]

[Andrzej Pietrasiewicz]

Hi all,

This is the second version of patches in this thread.

The series fixes problems with enumerating of SuperSpeed devices
on an Odroid XU3. There was a patch series from Vivek Gautam in
circulation, but it got lost somehow. Please see:

https://lkml.org/lkml/2014/9/2/166
https://lkml.org/lkml/2015/2/2/257

I adapted his patch so that it does not use a hacky solution to force
additional initialization in order for calibration to happen.
With this patch enumeration happens correctly and a super speed device
is recognized as such.

@Kishon:

As far as I understand what you suggest is to move phy_init() and
phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
to the best of my knowledge, they are needed in device mode, too.

Changes since v1:

- added calibrate() callback to phy
- used calibrate() instead of reset() to trigger the calibration

Andrzej Pietrasiewicz (1):
  drivers: phy: add calibrate method

Vivek Gautam (1):
  phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

 drivers/phy/phy-core.c                   |  14 +++
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 +++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.c                  |   7 +-
 include/linux/phy/phy.h                  |  10 ++
 4 files changed, 212 insertions(+), 2 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 0/2]

[Andrzej Pietrasiewicz]

Hi all,

This is the second version of patches in this thread.

The series fixes problems with enumerating of SuperSpeed devices
on an Odroid XU3. There was a patch series from Vivek Gautam in
circulation, but it got lost somehow. Please see:

https://lkml.org/lkml/2014/9/2/166
https://lkml.org/lkml/2015/2/2/257

I adapted his patch so that it does not use a hacky solution to force
additional initialization in order for calibration to happen.
With this patch enumeration happens correctly and a super speed device
is recognized as such.

@Kishon:

As far as I understand what you suggest is to move phy_init() and
phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
to the best of my knowledge, they are needed in device mode, too.

Changes since v1:

- added calibrate() callback to phy
- used calibrate() instead of reset() to trigger the calibration

Andrzej Pietrasiewicz (1):
  drivers: phy: add calibrate method

Vivek Gautam (1):
  phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800

 drivers/phy/phy-core.c                   |  14 +++
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 183 +++++++++++++++++++++++++++++++
 drivers/usb/dwc3/core.c                  |   7 +-
 include/linux/phy/phy.h                  |  10 ++
 4 files changed, 212 insertions(+), 2 deletions(-)

-- 
1.9.1

Re: [PATCHv2 0/2]

[Andrzej Pietrasiewicz]

W dniu 03.10.2017 o 14:59, Andrzej Pietrasiewicz pisze:
> Hi all,
> 
> This is the second version of patches in this thread.
> 

I have lost the subject of the cover letter.
Should be "dwc3 on XU3". I did not resend the series in order
not to spam the recipients. Sorry for confusion.

Andrzej

[PATCHv2 0/2]

[Andrzej Pietrasiewicz]

W dniu 03.10.2017 o?14:59, Andrzej Pietrasiewicz pisze:
> Hi all,
> 
> This is the second version of patches in this thread.
> 

I have lost the subject of the cover letter.
Should be "dwc3 on XU3". I did not resend the series in order
not to spam the recipients. Sorry for confusion.

Andrzej

Re: [PATCHv2 0/2]

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 October 2017 06:29 PM, Andrzej Pietrasiewicz wrote:
> Hi all,
> 
> This is the second version of patches in this thread.
> 
> The series fixes problems with enumerating of SuperSpeed devices
> on an Odroid XU3. There was a patch series from Vivek Gautam in
> circulation, but it got lost somehow. Please see:
> 
> https://lkml.org/lkml/2014/9/2/166
> https://lkml.org/lkml/2015/2/2/257
> 
> I adapted his patch so that it does not use a hacky solution to force
> additional initialization in order for calibration to happen.
> With this patch enumeration happens correctly and a super speed device
> is recognized as such.
> 
> @Kishon:
> 
> As far as I understand what you suggest is to move phy_init() and
> phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
> to the best of my knowledge, they are needed in device mode, too.

What I meant is perform phy initializations for host mode in xhci and keep only
device mode phy initialization in dwc3.

Thanks
Kishon

[PATCHv2 0/2]

[Kishon Vijay Abraham I]

Hi,

On Tuesday 03 October 2017 06:29 PM, Andrzej Pietrasiewicz wrote:
> Hi all,
> 
> This is the second version of patches in this thread.
> 
> The series fixes problems with enumerating of SuperSpeed devices
> on an Odroid XU3. There was a patch series from Vivek Gautam in
> circulation, but it got lost somehow. Please see:
> 
> https://lkml.org/lkml/2014/9/2/166
> https://lkml.org/lkml/2015/2/2/257
> 
> I adapted his patch so that it does not use a hacky solution to force
> additional initialization in order for calibration to happen.
> With this patch enumeration happens correctly and a super speed device
> is recognized as such.
> 
> @Kishon:
> 
> As far as I understand what you suggest is to move phy_init() and
> phy_power_on() invocations from dwc3/core.c to xhci-plat, but,
> to the best of my knowledge, they are needed in device mode, too.

What I meant is perform phy initializations for host mode in xhci and keep only
device mode phy initialization in dwc3.

Thanks
Kishon