Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -next] imx7: fix Kconfig warning and build errors
From: Randy Dunlap @ 2017-04-12 18:22 UTC (permalink / raw)
  To: linux-arm-kernel

From: Randy Dunlap <rdunlap@infradead.org>

Fix Kconfig warning in drivers/soc/imx/Kconfig and subsequent build errors
elsewhere when CONFIG_PM is not enabled.

warning: (IMX7_PM_DOMAINS) selects PM_GENERIC_DOMAINS which has unmet direct dependencies (PM)

This warning causes multiple build errors in drivers/base/power/domain.c:

../drivers/base/power/domain.c: In function 'genpd_queue_power_off_work':
../drivers/base/power/domain.c:279:13: error: 'pm_wq' undeclared (first use in this function)
  queue_work(pm_wq, &genpd->power_off_work);
../drivers/base/power/domain.c: In function 'genpd_dev_pm_qos_notifier':
../drivers/base/power/domain.c:462:25: error: 'struct dev_pm_info' has no member named 'ignore_children'
   if (!dev || dev->power.ignore_children)
../drivers/base/power/domain.c: In function 'rtpm_status_str':
../drivers/base/power/domain.c:2207:16: error: 'struct dev_pm_info' has no member named 'runtime_error'
  if (dev->power.runtime_error)
../drivers/base/power/domain.c:2209:21: error: 'struct dev_pm_info' has no member named 'disable_depth'
  else if (dev->power.disable_depth)
../drivers/base/power/domain.c:2211:21: error: 'struct dev_pm_info' has no member named 'runtime_status'
  else if (dev->power.runtime_status < ARRAY_SIZE(status_lookup))
   p = status_lookup[dev->power.runtime_status];

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc:	Shawn Guo <shawnguo@kernel.org>
Cc:	Sascha Hauer <kernel@pengutronix.de>
Cc:	Fabio Estevam <fabio.estevam@nxp.com>
Cc:	linux-arm-kernel at lists.infradead.org
---
 drivers/soc/imx/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20170412.orig/drivers/soc/imx/Kconfig
+++ linux-next-20170412/drivers/soc/imx/Kconfig
@@ -2,6 +2,7 @@ menu "i.MX SoC drivers"
 
 config IMX7_PM_DOMAINS
 	bool "i.MX7 PM domains"
+	depends on PM
 	select PM_GENERIC_DOMAINS
 	depends on SOC_IMX7D || (COMPILE_TEST && OF)
 	default y if SOC_IMX7D

^ permalink raw reply

* [PATCH -next] imx7: fix Kconfig warning and build errors
From: Fabio Estevam @ 2017-04-12 18:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <47fc2167-bf0a-866a-e075-8247a10aa6e0@infradead.org>

On Wed, Apr 12, 2017 at 3:22 PM, Randy Dunlap <rdunlap@infradead.org> wrote:

> --- linux-next-20170412.orig/drivers/soc/imx/Kconfig
> +++ linux-next-20170412/drivers/soc/imx/Kconfig
> @@ -2,6 +2,7 @@ menu "i.MX SoC drivers"
>
>  config IMX7_PM_DOMAINS
>         bool "i.MX7 PM domains"
> +       depends on PM
>         select PM_GENERIC_DOMAINS

Could this be:

select PM_GENERIC_DOMAINS if PM instead?

^ permalink raw reply

* kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds
From: Marc Zyngier @ 2017-04-12 18:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAeHK+x8udHKq9xa1zkTO6ax5E8Dk32HYWfaT05FMchL2cr48g@mail.gmail.com>

On 12/04/17 17:19, Andrey Konovalov wrote:

Hi Andrey,

> Apparently this wasn't fixed, I've got this report again on
> linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
> arm/arm64: Fix locking for kvm_free_stage2_pgd".

This looks like a different bug.

> 
> I now have a way to reproduce it, so I can test proposed patches. I
> don't have a simple C reproducer though.
> 
> The bug happens when the following syzkaller program is executed:
> 
> mmap(&(0x7f0000000000/0xc000)=nil, (0xc000), 0x3, 0x32, 0xffffffffffffffff, 0x0)
> unshare(0x400)
> perf_event_open(&(0x7f000002f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0xffffffff,
> 0xffffffffffffffff, 0x0)
> r0 = openat$kvm(0xffffffffffffff9c,
> &(0x7f000000c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
> ioctl$TIOCSBRK(0xffffffffffffffff, 0x5427)
> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
> syz_kvm_setup_cpu$arm64(r1, 0xffffffffffffffff,
> &(0x7f0000dc6000/0x18000)=nil, &(0x7f000000c000)=[{0x0,
> &(0x7f000000c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
> 0x32}], 0x1, 0x0, &(0x7f000000d000-0x10)=[@featur2={0x1, 0x3}], 0x1)

Is that the only thing the program does? Or is there anything running in
parallel?

> ==================================================================
> BUG: KASAN: use-after-free in arch_spin_is_locked
> include/linux/compiler.h:254 [inline]
> BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> Read of size 8 at addr ffff800004476730 by task syz-executor/13106
> 
> CPU: 1 PID: 13106 Comm: syz-executor Not tainted
> 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
> Hardware name: Hardkernel ODROID-C2 (DT)
> Call trace:
> [<ffff20000808fd08>] dump_backtrace+0x0/0x440 arch/arm64/kernel/traps.c:505
> [<ffff2000080903c0>] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
> [<ffff2000088df030>] __dump_stack lib/dump_stack.c:16 [inline]
> [<ffff2000088df030>] dump_stack+0x110/0x168 lib/dump_stack.c:52
> [<ffff200008406db8>] print_address_description+0x60/0x248 mm/kasan/report.c:252
> [<ffff2000084072c8>] kasan_report_error mm/kasan/report.c:351 [inline]
> [<ffff2000084072c8>] kasan_report+0x218/0x300 mm/kasan/report.c:408
> [<ffff200008407428>] __asan_report_load8_noabort+0x18/0x20 mm/kasan/report.c:429
> [<ffff2000080db1b8>] arch_spin_is_locked include/linux/compiler.h:254 [inline]

This is the assert on the spinlock, and the memory is gone.

> [<ffff2000080db1b8>] unmap_stage2_range+0x990/0x9a8
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
> [<ffff2000080db248>] kvm_free_stage2_pgd.part.16+0x30/0x98
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
> [<ffff2000080ddfb8>] kvm_free_stage2_pgd
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]

But we've taken than lock here. There's only a handful of instructions
in between, and the memory can only go away if there is something
messing with us in parallel.

> [<ffff2000080ddfb8>] kvm_arch_flush_shadow_all+0x40/0x58
> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
> [<ffff2000080c379c>] kvm_mmu_notifier_release+0x154/0x1d0
> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
> [<ffff2000083f2b60>] __mmu_notifier_release+0x1c0/0x3e0 mm/mmu_notifier.c:75
> [<ffff2000083a1fb4>] mmu_notifier_release
> include/linux/mmu_notifier.h:235 [inline]
> [<ffff2000083a1fb4>] exit_mmap+0x21c/0x288 mm/mmap.c:2941
> [<ffff20000810ecd4>] __mmput kernel/fork.c:888 [inline]
> [<ffff20000810ecd4>] mmput+0xdc/0x2e0 kernel/fork.c:910
> [<ffff20000811fda8>] exit_mm kernel/exit.c:557 [inline]
> [<ffff20000811fda8>] do_exit+0x648/0x2020 kernel/exit.c:865
> [<ffff2000081218b4>] do_group_exit+0xdc/0x260 kernel/exit.c:982
> [<ffff20000813adf0>] get_signal+0x358/0xf58 kernel/signal.c:2318
> [<ffff20000808de98>] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
> [<ffff20000808edb4>] do_notify_resume+0xe4/0x120 arch/arm64/kernel/signal.c:421
> [<ffff200008083e68>] work_pending+0x8/0x14

So we're being serviced with a signal. Do you know if this signal is
generated by your syzkaller program? We could be racing between do_exit
triggered by a fatal signal (this trace) and the closing of the two file
descriptors (vcpu and vm).

Paolo: does this look possible to you? I can't see what locking we have
that could prevent this race.

Thanks,
	
M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* kvm/arm64: use-after-free in kvm_unmap_hva_handler/unmap_stage2_pmds
From: Andrey Konovalov @ 2017-04-12 18:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1050c9d8-5813-5df9-29e5-3ab6e61b5de6@arm.com>

On Wed, Apr 12, 2017 at 8:43 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 12/04/17 17:19, Andrey Konovalov wrote:
>
> Hi Andrey,
>
>> Apparently this wasn't fixed, I've got this report again on
>> linux-next-c4e7b35a3 (Apr 11), which includes 8b3405e34 "kvm:
>> arm/arm64: Fix locking for kvm_free_stage2_pgd".
>
> This looks like a different bug.

Oh, OK.

>
>>
>> I now have a way to reproduce it, so I can test proposed patches. I
>> don't have a simple C reproducer though.
>>
>> The bug happens when the following syzkaller program is executed:
>>
>> mmap(&(0x7f0000000000/0xc000)=nil, (0xc000), 0x3, 0x32, 0xffffffffffffffff, 0x0)
>> unshare(0x400)
>> perf_event_open(&(0x7f000002f000-0x78)={0x1, 0x78, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x6, 0x0, 0x0, 0xd34, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
>> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0, 0xffffffff,
>> 0xffffffffffffffff, 0x0)
>> r0 = openat$kvm(0xffffffffffffff9c,
>> &(0x7f000000c000-0x9)="2f6465762f6b766d00", 0x0, 0x0)
>> ioctl$TIOCSBRK(0xffffffffffffffff, 0x5427)
>> r1 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0)
>> syz_kvm_setup_cpu$arm64(r1, 0xffffffffffffffff,
>> &(0x7f0000dc6000/0x18000)=nil, &(0x7f000000c000)=[{0x0,
>> &(0x7f000000c000)="5ba3c16f533efbed09f8221253c73763327fadce2371813b45dd7f7982f84a873e4ae89a6c2bd1af83a6024c36a1ff518318",
>> 0x32}], 0x1, 0x0, &(0x7f000000d000-0x10)=[@featur2={0x1, 0x3}], 0x1)
>
> Is that the only thing the program does? Or is there anything running in
> parallel?

These calls are executed repeatedly and in random order. That's all.

Except that I'm running the reproducer on a real arm board, so there's
probably a bunch of stuff going on besides these calls.

>
>> ==================================================================
>> BUG: KASAN: use-after-free in arch_spin_is_locked
>> include/linux/compiler.h:254 [inline]
>> BUG: KASAN: use-after-free in unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> Read of size 8 at addr ffff800004476730 by task syz-executor/13106
>>
>> CPU: 1 PID: 13106 Comm: syz-executor Not tainted
>> 4.11.0-rc6-next-20170411-xc2-11025-gc4e7b35a33d4-dirty #5
>> Hardware name: Hardkernel ODROID-C2 (DT)
>> Call trace:
>> [<ffff20000808fd08>] dump_backtrace+0x0/0x440 arch/arm64/kernel/traps.c:505
>> [<ffff2000080903c0>] show_stack+0x20/0x30 arch/arm64/kernel/traps.c:228
>> [<ffff2000088df030>] __dump_stack lib/dump_stack.c:16 [inline]
>> [<ffff2000088df030>] dump_stack+0x110/0x168 lib/dump_stack.c:52
>> [<ffff200008406db8>] print_address_description+0x60/0x248 mm/kasan/report.c:252
>> [<ffff2000084072c8>] kasan_report_error mm/kasan/report.c:351 [inline]
>> [<ffff2000084072c8>] kasan_report+0x218/0x300 mm/kasan/report.c:408
>> [<ffff200008407428>] __asan_report_load8_noabort+0x18/0x20 mm/kasan/report.c:429
>> [<ffff2000080db1b8>] arch_spin_is_locked include/linux/compiler.h:254 [inline]
>
> This is the assert on the spinlock, and the memory is gone.
>
>> [<ffff2000080db1b8>] unmap_stage2_range+0x990/0x9a8
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:295
>> [<ffff2000080db248>] kvm_free_stage2_pgd.part.16+0x30/0x98
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:842
>> [<ffff2000080ddfb8>] kvm_free_stage2_pgd
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:838 [inline]
>
> But we've taken than lock here. There's only a handful of instructions
> in between, and the memory can only go away if there is something
> messing with us in parallel.
>
>> [<ffff2000080ddfb8>] kvm_arch_flush_shadow_all+0x40/0x58
>> arch/arm64/kvm/../../../arch/arm/kvm/mmu.c:1895
>> [<ffff2000080c379c>] kvm_mmu_notifier_release+0x154/0x1d0
>> arch/arm64/kvm/../../../virt/kvm/kvm_main.c:472
>> [<ffff2000083f2b60>] __mmu_notifier_release+0x1c0/0x3e0 mm/mmu_notifier.c:75
>> [<ffff2000083a1fb4>] mmu_notifier_release
>> include/linux/mmu_notifier.h:235 [inline]
>> [<ffff2000083a1fb4>] exit_mmap+0x21c/0x288 mm/mmap.c:2941
>> [<ffff20000810ecd4>] __mmput kernel/fork.c:888 [inline]
>> [<ffff20000810ecd4>] mmput+0xdc/0x2e0 kernel/fork.c:910
>> [<ffff20000811fda8>] exit_mm kernel/exit.c:557 [inline]
>> [<ffff20000811fda8>] do_exit+0x648/0x2020 kernel/exit.c:865
>> [<ffff2000081218b4>] do_group_exit+0xdc/0x260 kernel/exit.c:982
>> [<ffff20000813adf0>] get_signal+0x358/0xf58 kernel/signal.c:2318
>> [<ffff20000808de98>] do_signal+0x170/0xc10 arch/arm64/kernel/signal.c:370
>> [<ffff20000808edb4>] do_notify_resume+0xe4/0x120 arch/arm64/kernel/signal.c:421
>> [<ffff200008083e68>] work_pending+0x8/0x14
>
> So we're being serviced with a signal. Do you know if this signal is
> generated by your syzkaller program? We could be racing between do_exit
> triggered by a fatal signal (this trace) and the closing of the two file
> descriptors (vcpu and vm).

I'm not sure.

>
> Paolo: does this look possible to you? I can't see what locking we have
> that could prevent this race.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v5 04/10] drm/stm: Add STM32 LTDC driver
From: Benjamin Gaignard @ 2017-04-12 18:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87wpaqeks2.fsf@eliezer.anholt.net>

2017-04-11 22:45 GMT+02:00 Eric Anholt <eric@anholt.net>:
> Yannick Fertre <yannick.fertre@st.com> writes:
>
>> This controller provides output signals to interface directly a variety
>> of LCD and TFT panels. These output signals are: RGB signals
>> (up to 24bpp), vertical & horizontal synchronisations, data enable and
>> the pixel clock.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
>> ---
>>  drivers/gpu/drm/Kconfig      |    3 +-
>>  drivers/gpu/drm/Makefile     |    1 +
>>  drivers/gpu/drm/stm/Kconfig  |   16 +
>>  drivers/gpu/drm/stm/Makefile |    7 +
>>  drivers/gpu/drm/stm/drv.c    |  221 ++++++++
>>  drivers/gpu/drm/stm/ltdc.c   | 1210 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/stm/ltdc.h   |   40 ++
>>  7 files changed, 1497 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/stm/Kconfig
>>  create mode 100644 drivers/gpu/drm/stm/Makefile
>>  create mode 100644 drivers/gpu/drm/stm/drv.c
>>  create mode 100644 drivers/gpu/drm/stm/ltdc.c
>>  create mode 100644 drivers/gpu/drm/stm/ltdc.h
>>
>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> index 78d7fc0..dd5762a 100644
>> --- a/drivers/gpu/drm/Kconfig
>> +++ b/drivers/gpu/drm/Kconfig
>> @@ -203,7 +203,6 @@ config DRM_VGEM
>>         as used by Mesa's software renderer for enhanced performance.
>>         If M is selected the module will be called vgem.
>>
>> -
>
> Stray whitespace change.
>
> With this removed, the driver is:
>
> Reviewed-by: Eric Anholt <eric@anholt.net>
>
> Apologies for the delay in the second review offered.  The remainder of
> my comments are little cleanups, all of which I think are optional and
> fine to do after the code lands.
>
> You should probably update MAINTAINERS for your new driver.  If you'd
> like to maintain this driver in the drm-misc small drivers collection
> (https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html),
> send a follow-up patch to the list to add the MAINTAINERS entry, and I
> can get that and patches 1-4 merged.  Once you have a few more patches
> in, we can add you to the drm-misc committers crew so you can merge
> directly after getting review.

Since we are moving sti driver to drm-misc it makes sense to move all
STMicroelectronic SoC display drivers to it.

>
> I'll also take this moment to plug something: Please feel welcome to
> review other people's driver patches on the list.  You've built
> something nice here, and probably learned a lot of lessons along the way
> that you could share with others.  (I just found out about
> of_reset_control in reviewing your code, and I wish I had known about it
> back when I was landing vc4!)
>
>> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
>> new file mode 100644
>> index 0000000..922f021
>> --- /dev/null
>> +++ b/drivers/gpu/drm/stm/ltdc.c
>
>> +static void ltdc_crtc_disable(struct drm_crtc *crtc)
>> +{
>> +     struct ltdc_device *ldev = crtc_to_ltdc(crtc);
>> +     struct drm_pending_vblank_event *event = crtc->state->event;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     if (!crtc->enabled) {
>> +             DRM_DEBUG_DRIVER("already disabled\n");
>> +             return;
>> +     }
>
> I think this crtc->enabled is a given for the disable() being called.
>
>> +
>> +     drm_crtc_vblank_off(crtc);
>> +
>> +     /* disable LTDC */
>> +     reg_clear(ldev->regs, LTDC_GCR, GCR_LTDCEN);
>> +
>> +     /* disable IRQ */
>> +     reg_clear(ldev->regs, LTDC_IER, IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> +     /* immediately commit disable of layers before switching off LTDC */
>> +     reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
>> +
>> +     if (event) {
>> +             crtc->state->event = NULL;
>> +
>> +             spin_lock_irq(&crtc->dev->event_lock);
>> +             if (crtc->state->active && drm_crtc_vblank_get(crtc) == 0)
>> +                     drm_crtc_arm_vblank_event(crtc, event);
>> +             else
>> +                     drm_crtc_send_vblank_event(crtc, event);
>> +             spin_unlock_irq(&crtc->dev->event_lock);
>> +     }
>
> I believe that we're guaranteed that crtc->state->event is NULL in the
> disable call, since your atomic_flush() already armed or sent the event
> and NULLed out the pointer.
>
>> +struct drm_connector *ltdc_rgb_connector_create(struct drm_device *ddev)
>> +{
>> +     struct drm_connector *connector;
>> +     int err;
>> +
>> +     connector = devm_kzalloc(ddev->dev, sizeof(*connector), GFP_KERNEL);
>> +     if (!connector) {
>> +             DRM_ERROR("Failed to allocate connector\n");
>> +             return NULL;
>> +     }
>> +
>> +     connector->polled = DRM_CONNECTOR_POLL_HPD;
>> +
>> +     err = drm_connector_init(ddev, connector, &ltdc_rgb_connector_funcs,
>> +                              DRM_MODE_CONNECTOR_LVDS);
>
> I think DRM_MODE_CONNECTOR_DPI (and _ENCODER_DPI) are slightly more
> accurate descriptions, if I'm interpreting your pinmux setup right.
> It's cosmetic, though.
>
>> +static struct drm_panel *ltdc_get_panel(struct drm_device *ddev)
>> +{
>> +     struct device *dev = ddev->dev;
>> +     struct device_node *np = dev->of_node;
>> +     struct device_node *entity, *port = NULL;
>> +     struct drm_panel *panel = NULL;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     /*
>> +      * Parse ltdc node to get remote port and find RGB panel / HDMI slave
>> +      * If a dsi or a bridge (hdmi, lvds...) is connected to ltdc,
>> +      * a remote port & RGB panel will not be found.
>> +      */
>> +     for_each_endpoint_of_node(np, entity) {
>> +             if (!of_device_is_available(entity))
>> +                     continue;
>> +
>> +             port = of_graph_get_remote_port_parent(entity);
>> +             if (port) {
>> +                     panel = of_drm_find_panel(port);
>> +                     of_node_put(port);
>> +                     if (panel) {
>> +                             DRM_DEBUG_DRIVER("remote panel %s\n",
>> +                                              port->full_name);
>> +                     } else {
>> +                             DRM_DEBUG_DRIVER("panel missing\n");
>> +                             of_node_put(entity);
>> +                     }
>> +             }
>> +     }
>
> Future work: You may find the new drm_of_find_panel_or_bridge() useful
> to drop this loop.
>
>> +
>> +     return panel;
>> +}
>> +
>> +int ltdc_load(struct drm_device *ddev)
>> +{
>> +     struct platform_device *pdev = to_platform_device(ddev->dev);
>> +     struct ltdc_device *ldev = ddev->dev_private;
>> +     struct device *dev = ddev->dev;
>> +     struct device_node *np = dev->of_node;
>> +     struct drm_encoder *encoder;
>> +     struct drm_connector *connector = NULL;
>> +     struct drm_crtc *crtc;
>> +     struct reset_control *rstc;
>> +     struct resource res;
>> +     int irq, ret, i;
>> +
>> +     DRM_DEBUG_DRIVER("\n");
>> +
>> +     ldev->panel = ltdc_get_panel(ddev);
>> +     if (!ldev->panel)
>> +             return -EPROBE_DEFER;
>> +
>> +     rstc = of_reset_control_get(np, NULL);
>> +
>> +     mutex_init(&ldev->err_lock);
>> +
>> +     ldev->pixel_clk = devm_clk_get(dev, "lcd");
>> +     if (IS_ERR(ldev->pixel_clk)) {
>> +             DRM_ERROR("Unable to get lcd clock\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     if (clk_prepare_enable(ldev->pixel_clk)) {
>> +             DRM_ERROR("Unable to prepare pixel clock\n");
>> +             return -ENODEV;
>> +     }
>
> Future work: You may want to move the pixel clock enable into the CRTC's
> .enable() and disable in .disable().  It sounded in previous versions
> like the HW uses that clock for all register accesses, so you'd need to
> protect a couple of other places, but that should save power when the
> device is off, right?
>
>> +
>> +     if (of_address_to_resource(np, 0, &res)) {
>> +             DRM_ERROR("Unable to get resource\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     ldev->regs = devm_ioremap_resource(dev, &res);
>> +     if (IS_ERR(ldev->regs)) {
>> +             DRM_ERROR("Unable to get ltdc registers\n");
>> +             return PTR_ERR(ldev->regs);
>> +     }
>> +
>> +     for (i = 0; i < MAX_IRQ; i++) {
>> +             irq = platform_get_irq(pdev, i);
>> +             if (irq < 0)
>> +                     continue;
>> +
>> +             ret = devm_request_threaded_irq(dev, irq, ltdc_irq,
>> +                                             ltdc_irq_thread, IRQF_ONESHOT,
>> +                                             dev_name(dev), ddev);
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to register LTDC interrupt\n");
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     if (!IS_ERR(rstc))
>> +             reset_control_deassert(rstc);
>> +
>> +     /* Disable interrupts */
>> +     reg_clear(ldev->regs, LTDC_IER,
>> +               IER_LIE | IER_RRIE | IER_FUIE | IER_TERRIE);
>> +
>> +     ret = ltdc_get_caps(ddev);
>> +     if (ret) {
>> +             DRM_ERROR("hardware identifier (0x%08x) not supported!\n",
>> +                       ldev->caps.hw_version);
>> +             return ret;
>> +     }
>> +
>> +     DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
>> +
>> +     if (ltdc_create_encoders(ddev)) {
>> +             DRM_ERROR("Failed to create encoders\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (ldev->panel) {
>> +             encoder = ltdc_rgb_encoder_find(ddev);
>> +             if (!encoder) {
>> +                     DRM_ERROR("Failed to find RGB encoder\n");
>> +                     ret = -EINVAL;
>> +                     goto err;
>> +             }
>
> Given that ltdc_create_encoders() only does work if ldev->panel, its
> body could probably be moved in here and then ltdc_rgb_encoder_find
> could be dropped.
>
>> +
>> +             connector = ltdc_rgb_connector_create(ddev);
>> +             if (!connector) {
>> +                     DRM_ERROR("Failed to create RGB connector\n");
>> +                     ret = -EINVAL;
>> +                     goto err;
>> +             }
>> +
>> +             ret = drm_mode_connector_attach_encoder(connector, encoder);
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to attach connector to encoder\n");
>> +                     goto err;
>> +             }
>> +
>> +             drm_panel_attach(ldev->panel, connector);
>> +     }
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org ? Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v4 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Bjorn Helgaas @ 2017-04-12 19:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412181034.GA3671@localhost>

On Wed, Apr 12, 2017 at 06:10:34PM +0000, Jayachandran C wrote:
> On Wed, Apr 12, 2017 at 11:21:18AM -0500, Bjorn Helgaas wrote:
> > On Tue, Apr 11, 2017 at 03:27:02PM +0000, Jayachandran C wrote:
> > > On Tue, Apr 11, 2017 at 08:41:25AM -0500, Bjorn Helgaas wrote:
> > > > [+cc Joerg]
> > > > 
> > > > On Tue, Apr 11, 2017 at 07:10:48AM +0000, Jayachandran C wrote:
> > > > > On Mon, Apr 10, 2017 at 08:28:47PM -0500, Bjorn Helgaas wrote:
> > > > > > Hi Jayachandran,
> > > > > > 
> > > > > > On Mon, Apr 03, 2017 at 01:15:04PM +0000, Jayachandran C wrote:
> > > > > > > The Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the PCI
> > > > > > > topology is slightly unusual. For a multi-node system, it looks like:
> > > > > > > 
> > > > > > > [node level PCI bridges - one per node]
> > > > > > >     [SoC PCI devices with MSI-X but no IOMMU]
> > > > > > >     [PCI-PCIe "glue" bridges - upto 14, one per real port below]
> > > > > > >         [PCIe real root ports associated with IOMMU and GICv3 ITS]
> > > > > > >             [External PCI devices connected to PCIe links]
> > > > > > > 
> > > > > > > The top two levels of bridges should have introduced aliases since they
> > > > > > > are PCI and PCI/PCIe bridges, but in the case of ThunderX2 they do not.
> > > > > > > In the case of external PCIe devices, the "real" root ports are connected
> > > > > > > to the SMMU and the GIC ITS, so PCI-PCIe bridge does not introduce an
> > > > > > > alias. The SoC PCI devices are directly connected to the GIC ITS, so the
> > > > > > > node level bridges do not introduce an alias either.
> > > > > > > 
> > > > > > > To handle this quirk, we mark the real PCIe root ports and node level
> > > > > > > PCI bridges with the flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT.  With this,
> > > > > > > pci_for_each_dma_alias() works correctly for external PCIe devices and
> > > > > > > SoC PCI devices.
> > > > > > > 
> > > > > > > For the current revision of Cavium ThunderX2, the VendorID and Device ID
> > > > > > > are from Broadcom Vulcan (14e4:90XX).
> > > > > > 
> > > > > > Can you supply some text here about why we want to apply this patch?
> > > > > > E.g., does it avoid making unnecessary IOMMU mappings, improve
> > > > > > performance, avoid a crash, etc?
> > > > > 
> > > > > If this is for the commit message, I hope the following is ok:
> > > > > 
> > > > > "With this change, both MSI-X and IO virtualization work correctly on
> > > > > Cavium ThunderX2. The GIC ITS driver gets the correct device ID to
> > > > > configure MSI-X, the SMMUv3 driver gets the correct Stream IDs for PCI
> > > > > devices, and the IOMMU groups are setup correctly."
> > > > 
> > > > This doesn't get at what the actual problem is.  I'm hoping for
> > > > something like "without this change, we set up an IOMMU mapping for
> > > > requestor ID X, but device DMA uses requestor ID Y because ...., which
> > > > results in an IOMMU fault"
> > > 
> > > Ok. I hope this would be better:
> > > 
> > > "Without this change, the last alias seen while traversing the PCI
> > > hierarchy will be used as the RID to generate the device ID for ITS
> > > and stream ID for SMMU. This in turn causes the MSI-X generated by the
> > > device to fail since the ITS expects to have translation tables based
> > > on the actual PCIe RID and not the (irrelevant) alias. Similarly, the
> > > device DMA also fails when SMMU is enabled due to incorrect value in
> > > SMMU translation tables"
> > 
> > This description is true, but I don't think it addresses the real
> > problem.  I think the real problem is that your IOMMU code doesn't
> > handle aliases correctly, and by ignoring these invalid aliases, we
> > happen to map an alias that works for the builtin devices.  But that's
> > only because we got lucky (those devices use a single RID and they're
> > not behind bridges that optionally take ownership).
> > 
> > It would make sense to me if we fixed the IOMMU code to map *all* the
> > aliases, which should be enough to make your devices work.  If we then
> > wanted to apply a patch like this on top, it would be simply an
> > optimization that avoids unnecessary IOMMU mappings.
> 
> The issue that the IOMMU code does not handle valid aliases is
> unrelated to what I am trying to fix. The quirk is to make sure
> that invalid aliases are not seen on ThunderX2 while doing
> pci_for_each_dma_alias().
> 
> The DMA and MSI-X requests leave the PCI/PCIe hierarchy at the point
> where the SMMU (or ITS) is attached, i.e. at the bridge marked with
> PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT. The quirk ensures that we don't look
> for aliases above that point.
> 
> The toplevel bridge is marked PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT since
> the on-chip devices are directly connected to the ITS (they do not
> use SMMU).
> 
> The top two levels of bridges are not real PCI bridges but just
> PCI bridge-like things that were added to tie the whole hierarchy
> together for configuration and enumeration. They do not handle
> PCI/PCIe transactions in the traditional sense.
> 
> I think my problem description is still not correct, maybe:
> "The SMMU (and ITS) expects to device tables to use the RID seen
> at the bridge they are associated with. Currently the
> pci_for_each_dma_alias() code traverses beyond this point and
> generates incorrect aliases due to the PCI and PCI/PCIe bridges
> above. This causes MSI-X interrupts and device DMA to fail since
> the SMMU and ITS tables to be setup with incorrect IDs"

I haven't tried to figure out the MSI-X piece of this, but let me try
to come up with a concrete DMA example.  Assume this topology:

  00:00.0 bridge to [bus 01-1e]
  01:0a.0 bridge to [bus 04-05]
  04:00.0 [14e4:9000 or 9084] bridge to [bus 05] (XLATE_ROOT)
  05:00.0 endpoint

Assume 05:00.0 generates a DMA request.  Assume the top two bridges
are such that pci_for_each_dma_alias() includes them as well, so it
iterates through 05:00.0, 01:0a.0, and 00:00.0.

When the driver for 05:00.0 makes a DMA mapping, the current code
apparently makes an IOMMU mapping for requester ID 00:00.0 because
that's the last alias.  Obviously this doesn't work because the IOMMU
at 04:00.0 will see a requester ID of 05:00.0, not 00:00.0.

With this quirk, we'll omit 01:0a.0 and 00:00.0, so we'll make an
IOMMU mapping for requester ID 05:00.0, which will work fine.  I think
it would *also* work fine if we made IOMMU mappings for 05:00.0,
01:0a.0, and 00:0.0.  The last two are unnecessary, but probably not
harmful.

Now assume 05:00 is a multi-function device that has a DMA alias
quirk, e.g., see quirk_dma_func0_alias().  It has another function:

  05:00.3 endpoint

DMA from 05:00.3 may use a requester ID of either 05:00.3 or 05:00.0.
The driver makes a DMA mapping, pci_for_each_dma_alias() iterates
through 05:00.3, 05:00.0, 01:0a.0, and 00:00.0, and we again map only
00:00.0, which again doesn't work.

With this quirk, we create a single mapping for 05:00.0.  That will
work sometimes, but the device may also generate DMA with a requester
ID of 05:00.3, and that won't work.

If your on-chip device is, e.g., 01:04.0, pci_for_each_dma_alias()
probably iterates through 01:04.0, 00:00.0.  Today we make an IOMMU
mapping for 00:00.0, which doesn't work.  With this quirk, we'll
ignore 00:00.0 and make a mapping for 01:04.0, which does work.  But I
think if you made the IOMMU code add mappings for each of the aliases,
i.e., for both 01:04.0 and 00:00.0, that device *would* work even
without this quirk.

Bjorn

^ permalink raw reply

* [PATCH V8 2/5] PCI/ASPM: split pci_aspm_init() into two
From: Rajat Jain @ 2017-04-12 19:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1491627351-1111-3-git-send-email-okaya@codeaurora.org>

On Fri, Apr 7, 2017 at 9:55 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> Split pci_aspm_init() body into pci_aspm_init_upstream()
> and pci_aspm_init_downstream() for bridge and endpoint
> specific code behavior.
>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/pcie/aspm.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index dc36717..a80d64b 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -826,6 +826,16 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
>         return link;
>  }
>
> +static int pci_aspm_init_downstream(struct pci_dev *pdev)
> +{
> +       return 0;
> +}
> +
> +static int pci_aspm_init_upstream(struct pci_dev *pdev)
> +{
> +       return 0;
> +}
> +
>  /*
>   * pci_aspm_init: Initiate PCI express link state.
>   * It is called from device_add for every single pci device.
> @@ -833,7 +843,10 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
>   */
>  int pci_aspm_init(struct pci_dev *pdev)
>  {
> -       return 0;
> +       if (!pdev->has_secondary_link)
> +               return pci_aspm_init_downstream(pdev);
> +
> +       return pci_aspm_init_upstream(pdev);
>  }

Nit:

if (x_flag())
   return x();
return y();

May be better than

if (!x_flag)
    return y();
return x();


>
>  /*
> --
> 1.9.1
>

^ permalink raw reply

* [PATCH V8 4/5] PCI/ASPM: save power on values during bridge init
From: Rajat Jain @ 2017-04-12 19:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1491627351-1111-5-git-send-email-okaya@codeaurora.org>

On Fri, Apr 7, 2017 at 9:55 PM, Sinan Kaya <okaya@codeaurora.org> wrote:
> Now that we added a hook to be called from device_add, save the
> default values from the HW registers early in the boot for further
> reuse during hot device add/remove operations.
>
> If the link is down during boot, assume that we want to enable L0s
> and L1 following hotplug insertion as well as L1SS if supported.

IIUC, so far POLICY_DEFAULT meant that we'd just use & follow what
BIOS has done, and play it safe (never try to be more opportunistic).
With this change however, we'd be slightly overstepping and giving
ourselves benefit of doubt if the BIOS could not enable ASPM states
because the link was not up. This may be good, but I think we should
call it out, and add some more elaborate comment on the POLICY_DEFAULT
description (what to, and what not to expect in different situations).

It is important because existing systems today, that used to boot
without cards and later hotplugged them, didn't have ASPM states
enabled. They will now suddenly start seeing all ASPM states enabled
including L1 substates for the first time (if supported).

My system is not hotplug capable (I have the EP soldered on board, so
couldn't do much testing, except for sanity. Please feel free to use
my Reviewed-by.

>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=194895
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/pcie/aspm.c | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index e33f84b..c7da087 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -505,8 +505,10 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>          */
>         if (dwreg.support & upreg.support & PCIE_LINK_STATE_L0S)
>                 link->aspm_support |= ASPM_STATE_L0S;
> -       if (dwreg.enabled & PCIE_LINK_STATE_L0S)
> +       if (dwreg.enabled & PCIE_LINK_STATE_L0S) {
>                 link->aspm_enabled |= ASPM_STATE_L0S_UP;
> +               link->aspm_default |= ASPM_STATE_L0S_UP;
> +       }
>         if (upreg.enabled & PCIE_LINK_STATE_L0S)
>                 link->aspm_enabled |= ASPM_STATE_L0S_DW;
>         link->latency_up.l0s = calc_l0s_latency(upreg.latency_encoding_l0s);
> @@ -542,9 +544,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>         if (link->aspm_support & ASPM_STATE_L1SS)
>                 aspm_calc_l1ss_info(link, &upreg, &dwreg);
>
> -       /* Save default state */
> -       link->aspm_default = link->aspm_enabled;
> -
>         /* Setup initial capable state. Will be updated later */
>         link->aspm_capable = link->aspm_support;
>         /*
> @@ -835,11 +834,38 @@ static int pci_aspm_init_downstream(struct pci_dev *pdev)
>  static int pci_aspm_init_upstream(struct pci_dev *pdev)
>  {
>         struct pcie_link_state *link;
> +       struct aspm_register_info upreg;
> +       u16 lnk_status;
> +       bool ret;
>
>         link = alloc_pcie_link_state(pdev);
>         if (!link)
>                 return -ENOMEM;
>
> +       pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
> +       ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
> +
> +       if (ret) {
> +               pcie_get_aspm_reg(pdev, &upreg);
> +               if (upreg.enabled & PCIE_LINK_STATE_L0S)
> +                       link->aspm_default |= ASPM_STATE_L0S_DW;
> +               if (upreg.enabled & PCIE_LINK_STATE_L1)
> +                       link->aspm_default |= ASPM_STATE_L1;
> +               if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
> +                       link->aspm_default |= ASPM_STATE_L1_1;
> +               if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
> +                       link->aspm_default |= ASPM_STATE_L1_2;
> +               if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
> +                       link->aspm_default |= ASPM_STATE_L1_1_PCIPM;
> +               if (upreg.l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
> +                       link->aspm_default |= ASPM_STATE_L1_2_PCIPM;
> +       } else {
> +               if (!pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS))
> +                       link->aspm_default = ASPM_STATE_L0S | ASPM_STATE_L1;
> +               else
> +                       link->aspm_default = ASPM_STATE_ALL;
> +       }

Optional: May be consider moving this code (more aptly) to
pcie_aspm_cap_init() by adding a check for link-up before we start
reading downstream registers there? I guess you'll need to move the
call to pcie_aspm_cap_init() a little further up in
pcie_aspm_init_link_state().

> +
>         return 0;
>  }
>
> --
> 1.9.1
>

^ permalink raw reply

* [PATCH -next] imx7: fix Kconfig warning and build errors
From: Randy Dunlap @ 2017-04-12 19:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOMZO5CF4GZx2UWxq3SF_=g9hnSS9A28T4tPLaSHQr-qJs623Q@mail.gmail.com>

On 04/12/17 11:26, Fabio Estevam wrote:
> On Wed, Apr 12, 2017 at 3:22 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> 
>> --- linux-next-20170412.orig/drivers/soc/imx/Kconfig
>> +++ linux-next-20170412/drivers/soc/imx/Kconfig
>> @@ -2,6 +2,7 @@ menu "i.MX SoC drivers"
>>
>>  config IMX7_PM_DOMAINS
>>         bool "i.MX7 PM domains"
>> +       depends on PM
>>         select PM_GENERIC_DOMAINS
> 
> Could this be:
> 
> select PM_GENERIC_DOMAINS if PM instead?
> 

You want IMX7_PM_DOMAINS to be built without having
PM_GENERIC_DOMAINS enabled?

On x86, your suggestion works for me... because of this
in drivers/soc/Makefile:

obj-$(CONFIG_ARCH_MXC)		+= imx/

and I don't have CONFIG_ARCH_MXC, so the gpc* binaries are
not being built at all.  This might be a small bug.


-- 
~Randy

^ permalink raw reply

* [PATCH -next] imx7: fix Kconfig warning and build errors
From: Fabio Estevam @ 2017-04-12 19:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8f538f2f-320a-78bb-1008-8e27908f8cf9@infradead.org>

On Wed, Apr 12, 2017 at 4:24 PM, Randy Dunlap <rdunlap@infradead.org> wrote:

> You want IMX7_PM_DOMAINS to be built without having
> PM_GENERIC_DOMAINS enabled?
>
> On x86, your suggestion works for me... because of this
> in drivers/soc/Makefile:
>
> obj-$(CONFIG_ARCH_MXC)          += imx/
>
> and I don't have CONFIG_ARCH_MXC, so the gpc* binaries are
> not being built at all.  This might be a small bug.

Just tested your patch with imx_v6_v7_defconfig + manually doing
CONFIG_PM=n  and it does fix the build error, thanks:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply

* [PATCH v2 3/4] bluetooth: hci_uart: add LL protocol serdev driver support
From: Marcel Holtmann @ 2017-04-12 20:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170407143516.9945-1-robh@kernel.org>

Hi Rob,

> Turns out that the LL protocol and the TI-ST are the same thing AFAICT.
> The TI-ST adds firmware loading, GPIO control, and shared access for
> NFC, FM radio, etc. For now, we're only implementing what is needed for
> BT. This mirrors other drivers like BCM and Intel, but uses the new
> serdev bus.
> 
> The firmware loading is greatly simplified by using existing
> infrastructure to send commands. It may be a bit slower than the
> original code using synchronous functions, but the real bottleneck is
> likely doing firmware load at 115.2kbps.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> Cc: linux-bluetooth at vger.kernel.org
> ---
> v2:
> - Use IS_ENABLED() to fix module build
> 
> drivers/bluetooth/hci_ll.c | 261 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 260 insertions(+), 1 deletion(-)

can you re-send any missing patch on top of today's bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* [PATCH v4 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Jayachandran C @ 2017-04-12 20:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412191138.GH25197@bhelgaas-glaptop.roam.corp.google.com>

On Wed, Apr 12, 2017 at 02:11:38PM -0500, Bjorn Helgaas wrote:
> On Wed, Apr 12, 2017 at 06:10:34PM +0000, Jayachandran C wrote:
> > On Wed, Apr 12, 2017 at 11:21:18AM -0500, Bjorn Helgaas wrote:
> > > On Tue, Apr 11, 2017 at 03:27:02PM +0000, Jayachandran C wrote:
> > > > On Tue, Apr 11, 2017 at 08:41:25AM -0500, Bjorn Helgaas wrote:
> > > > > [+cc Joerg]
> > > > > 
> > > > > On Tue, Apr 11, 2017 at 07:10:48AM +0000, Jayachandran C wrote:
> > > > > > On Mon, Apr 10, 2017 at 08:28:47PM -0500, Bjorn Helgaas wrote:
> > > > > > > Hi Jayachandran,
> > > > > > > 
> > > > > > > On Mon, Apr 03, 2017 at 01:15:04PM +0000, Jayachandran C wrote:
> > > > > > > > The Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the PCI
> > > > > > > > topology is slightly unusual. For a multi-node system, it looks like:
> > > > > > > > 
> > > > > > > > [node level PCI bridges - one per node]
> > > > > > > >     [SoC PCI devices with MSI-X but no IOMMU]
> > > > > > > >     [PCI-PCIe "glue" bridges - upto 14, one per real port below]
> > > > > > > >         [PCIe real root ports associated with IOMMU and GICv3 ITS]
> > > > > > > >             [External PCI devices connected to PCIe links]
> > > > > > > > 
> > > > > > > > The top two levels of bridges should have introduced aliases since they
> > > > > > > > are PCI and PCI/PCIe bridges, but in the case of ThunderX2 they do not.
> > > > > > > > In the case of external PCIe devices, the "real" root ports are connected
> > > > > > > > to the SMMU and the GIC ITS, so PCI-PCIe bridge does not introduce an
> > > > > > > > alias. The SoC PCI devices are directly connected to the GIC ITS, so the
> > > > > > > > node level bridges do not introduce an alias either.
> > > > > > > > 
> > > > > > > > To handle this quirk, we mark the real PCIe root ports and node level
> > > > > > > > PCI bridges with the flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT.  With this,
> > > > > > > > pci_for_each_dma_alias() works correctly for external PCIe devices and
> > > > > > > > SoC PCI devices.
> > > > > > > > 
> > > > > > > > For the current revision of Cavium ThunderX2, the VendorID and Device ID
> > > > > > > > are from Broadcom Vulcan (14e4:90XX).
> > > > > > > 
> > > > > > > Can you supply some text here about why we want to apply this patch?
> > > > > > > E.g., does it avoid making unnecessary IOMMU mappings, improve
> > > > > > > performance, avoid a crash, etc?
> > > > > > 
> > > > > > If this is for the commit message, I hope the following is ok:
> > > > > > 
> > > > > > "With this change, both MSI-X and IO virtualization work correctly on
> > > > > > Cavium ThunderX2. The GIC ITS driver gets the correct device ID to
> > > > > > configure MSI-X, the SMMUv3 driver gets the correct Stream IDs for PCI
> > > > > > devices, and the IOMMU groups are setup correctly."
> > > > > 
> > > > > This doesn't get at what the actual problem is.  I'm hoping for
> > > > > something like "without this change, we set up an IOMMU mapping for
> > > > > requestor ID X, but device DMA uses requestor ID Y because ...., which
> > > > > results in an IOMMU fault"
> > > > 
> > > > Ok. I hope this would be better:
> > > > 
> > > > "Without this change, the last alias seen while traversing the PCI
> > > > hierarchy will be used as the RID to generate the device ID for ITS
> > > > and stream ID for SMMU. This in turn causes the MSI-X generated by the
> > > > device to fail since the ITS expects to have translation tables based
> > > > on the actual PCIe RID and not the (irrelevant) alias. Similarly, the
> > > > device DMA also fails when SMMU is enabled due to incorrect value in
> > > > SMMU translation tables"
> > > 
> > > This description is true, but I don't think it addresses the real
> > > problem.  I think the real problem is that your IOMMU code doesn't
> > > handle aliases correctly, and by ignoring these invalid aliases, we
> > > happen to map an alias that works for the builtin devices.  But that's
> > > only because we got lucky (those devices use a single RID and they're
> > > not behind bridges that optionally take ownership).
> > > 
> > > It would make sense to me if we fixed the IOMMU code to map *all* the
> > > aliases, which should be enough to make your devices work.  If we then
> > > wanted to apply a patch like this on top, it would be simply an
> > > optimization that avoids unnecessary IOMMU mappings.
> > 
> > The issue that the IOMMU code does not handle valid aliases is
> > unrelated to what I am trying to fix. The quirk is to make sure
> > that invalid aliases are not seen on ThunderX2 while doing
> > pci_for_each_dma_alias().
> > 
> > The DMA and MSI-X requests leave the PCI/PCIe hierarchy at the point
> > where the SMMU (or ITS) is attached, i.e. at the bridge marked with
> > PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT. The quirk ensures that we don't look
> > for aliases above that point.
> > 
> > The toplevel bridge is marked PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT since
> > the on-chip devices are directly connected to the ITS (they do not
> > use SMMU).
> > 
> > The top two levels of bridges are not real PCI bridges but just
> > PCI bridge-like things that were added to tie the whole hierarchy
> > together for configuration and enumeration. They do not handle
> > PCI/PCIe transactions in the traditional sense.
> > 
> > I think my problem description is still not correct, maybe:
> > "The SMMU (and ITS) expects to device tables to use the RID seen
> > at the bridge they are associated with. Currently the
> > pci_for_each_dma_alias() code traverses beyond this point and
> > generates incorrect aliases due to the PCI and PCI/PCIe bridges
> > above. This causes MSI-X interrupts and device DMA to fail since
> > the SMMU and ITS tables to be setup with incorrect IDs"
> 
> I haven't tried to figure out the MSI-X piece of this, but let me try
> to come up with a concrete DMA example.  Assume this topology:
> 
>   00:00.0 bridge to [bus 01-1e]
>   01:0a.0 bridge to [bus 04-05]
>   04:00.0 [14e4:9000 or 9084] bridge to [bus 05] (XLATE_ROOT)
>   05:00.0 endpoint
> 
> Assume 05:00.0 generates a DMA request.  Assume the top two bridges
> are such that pci_for_each_dma_alias() includes them as well, so it
> iterates through 05:00.0, 01:0a.0, and 00:00.0.
> 
> When the driver for 05:00.0 makes a DMA mapping, the current code
> apparently makes an IOMMU mapping for requester ID 00:00.0 because
> that's the last alias.  Obviously this doesn't work because the IOMMU
> at 04:00.0 will see a requester ID of 05:00.0, not 00:00.0.
> 
> With this quirk, we'll omit 01:0a.0 and 00:00.0, so we'll make an
> IOMMU mapping for requester ID 05:00.0, which will work fine.  I think
> it would *also* work fine if we made IOMMU mappings for 05:00.0,
> 01:0a.0, and 00:0.0.  The last two are unnecessary, but probably not
> harmful.
 
Ok. The last two are not harmful but still incorrect. The bridges
0:0.0 and 1:a.0 are outside the path with the PCI transactions takes
(they go from 4:0.0 to the SMMU).

> Now assume 05:00 is a multi-function device that has a DMA alias
> quirk, e.g., see quirk_dma_func0_alias().  It has another function:
> 
>   05:00.3 endpoint
> 
> DMA from 05:00.3 may use a requester ID of either 05:00.3 or 05:00.0.
> The driver makes a DMA mapping, pci_for_each_dma_alias() iterates
> through 05:00.3, 05:00.0, 01:0a.0, and 00:00.0, and we again map only
> 00:00.0, which again doesn't work.
> 
> With this quirk, we create a single mapping for 05:00.0.  That will
> work sometimes, but the device may also generate DMA with a requester
> ID of 05:00.3, and that won't work.

Note that here, pci_for_each_dma_alias() works correctly with the quirk
and incorrectly without the quirk. With the quirk, the callback function
is involved on 05:00.3 and 05:00.0 as expected. Without the quirk the
call back is invoked on 05:00.3, 05:00.0, 01:0a.0, and 00:00.0, which
includes aliases that are not valid.

The idea behind the quirk is to get pci_for_each_dma_alias work correctly
on ThunderX2, and invoke the function only on the valid aliases. With the
quirk, the code using it - like the SMMU code - gets the correct aliases
to process, and do not have to deal with or filter out incorrect aliases.

I don't think it would be safe to assume that all callers of
pci_for_each_dma_alias will be ok to handle invalid aliases. Even in
the IOMMU case, the case which calculates stream IDs is one usage, the
usage for iommu groups also did not deal with invalid aliases correctly
when I tried it. Then there are the MSI cases too, I had done some work
on trying to filter out invalid aliases in the relevant code[1], but
in my opinion, getting pci_for_each_dma_alias() do the right thing is
the correct solution.

> If your on-chip device is, e.g., 01:04.0, pci_for_each_dma_alias()
> probably iterates through 01:04.0, 00:00.0.  Today we make an IOMMU
> mapping for 00:00.0, which doesn't work.  With this quirk, we'll
> ignore 00:00.0 and make a mapping for 01:04.0, which does work.  But I
> think if you made the IOMMU code add mappings for each of the aliases,
> i.e., for both 01:04.0 and 00:00.0, that device *would* work even
> without this quirk.

There is no IOMMU for the on-chip devices, but the MSI-X interrupts are
looked up in a table based on DeviceID(RID) by the ARM interrupt
controller. The issue here is similar.

JC.

[1]https://www.spinics.net/lists/arm-kernel/msg573744.html

^ permalink raw reply

* [PATCH v1 1/1] mtd: mtk-nor: set controller's address width according to nor flash
From: Cyrille Pitchen @ 2017-04-12 20:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1491381462-21893-2-git-send-email-guochun.mao@mediatek.com>

Hi Guochun,

Le 05/04/2017 ? 10:37, Guochun Mao a ?crit :
> When nor's size larger than 16MByte, nor's address width maybe
> set to 3 or 4, and controller should change address width according
> to nor's setting.
> 
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> ---
>  drivers/mtd/spi-nor/mtk-quadspi.c |   27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> index e661877..b637770 100644
> --- a/drivers/mtd/spi-nor/mtk-quadspi.c
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -104,6 +104,8 @@
>  #define MTK_NOR_MAX_RX_TX_SHIFT		6
>  /* can shift up to 56 bits (7 bytes) transfer by MTK_NOR_PRG_CMD */
>  #define MTK_NOR_MAX_SHIFT		7
> +/* nor controller 4-byte address mode enable bit */
> +#define MTK_NOR_4B_ADDR_EN		BIT(4)
>  
>  /* Helpers for accessing the program data / shift data registers */
>  #define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
> @@ -230,10 +232,35 @@ static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
>  				  10000);
>  }
>  
> +static void mt8173_nor_set_addr_width(struct mt8173_nor *mt8173_nor)
> +{
> +	u8 val;
> +	struct spi_nor *nor = &mt8173_nor->nor;
> +
> +	val = readb(mt8173_nor->base + MTK_NOR_DUAL_REG);
> +
> +	switch (nor->addr_width) {
> +	case 3:
> +		val &= ~MTK_NOR_4B_ADDR_EN;
> +		break;
> +	case 4:
> +		val |= MTK_NOR_4B_ADDR_EN;
> +		break;
> +	default:
> +		dev_warn(mt8173_nor->dev, "Unexpected address width %u.\n",
> +			 nor->addr_width);
> +		break;
> +	}
> +
> +	writeb(val, mt8173_nor->base + MTK_NOR_DUAL_REG);
> +}
> +
>  static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
>  {
>  	int i;
>  
> +	mt8173_nor_set_addr_width(mt8173_nor);
> +
>  	for (i = 0; i < 3; i++) {

Should it be 'i < nor->addr_width' instead of 'i < 3' ?
Does it work when accessing data after 128Mbit ?

Best regards,

Cyrille

>  		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
>  		addr >>= 8;
> 

^ permalink raw reply

* [PATCH 5/8] arm64: defconfig: enable EFI_VARS and EFI_CAPSULE_LOADER
From: Timur Tabi @ 2017-04-12 20:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412161442.GB7153@leverpostej>

On 04/12/2017 11:14 AM, Mark Rutland wrote:
>>> > > Many ARM64 server systems boot via EFI, so enable support for some EFI
>>> > > options to allow applications to interact with EFI.
>>> > > 
>>> > > CONFIG_EFI_VARS enables sysfs support for reading and writing variables
>> > 
>> > Isn't efivarfs preferred for that these days, or have I imagined that?
>> > 
>> > Robin.
> Ard, Leif?

So my research suggests that efivarfs is a replacement for CONFIG_EFI_VARS.
We may still have some older programs that use the sysfs entries, but I'm
not sure.

Regardless, I will drop EFI_VARS from v2 of the patset, and just set
EFI_CAPSULE_LOADER.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* et8ek8 camera on Nokia N900: trying to understand what is going on with modes
From: Pavel Machek @ 2017-04-12 21:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hi!

5Mpix mode does not work on N900, which is something I'd like to
understand. et8ek8_mode contains huge tables of register settings and
parameter values, but it seems that they are not really independend.

To test that theory, I started with checking values against each
other.

This is the work so far, it is neither complete nor completely working
at the moment. Perhaps someone wants to play...

								Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 6296f6f..ca2f648 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -798,6 +798,8 @@ static void et8ek8_update_controls(struct et8ek8_sensor *sensor)
 	u32 min, max, pixel_rate;
 	static const int S = 8;
 
+	printk("Updating controls for %d x %d @ %d mode -- %s\n", mode->width, mode->height, mode->pixel_clock, mode->name);
+
 	ctrl = sensor->exposure;
 
 #ifdef COMPATIBLE
@@ -820,6 +822,127 @@ static void et8ek8_update_controls(struct et8ek8_sensor *sensor)
 	__v4l2_ctrl_s_ctrl_int64(sensor->pixel_rate, pixel_rate << S);
 }
 
+static int read_8(struct i2c_client *client, unsigned long addr)
+{
+	int val;
+	et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, addr, &val);
+	return val;
+}
+
+static int read_16(struct i2c_client *client, unsigned long addr)
+{
+	return read_8(client, addr);
+}
+
+static void assert_value(struct i2c_client *client, unsigned long addr, unsigned long val)
+{
+	int val2 = read_8(client, addr);
+	if (val != val2)
+		printk("et8ek8: assertion check %lx / should be %lx is %lx\n", addr, val, val2);
+}
+
+static void assert(struct i2c_client *client, int v, char *msg)
+{
+	if (!v)
+		printk("et8ek8: assertion: %s\n", msg);
+}
+
+static void assert_eq(struct i2c_client *client, int v1, int v2, char *msg)
+{
+	if (v1 != v2)
+		printk("et8ek8: assertion: %d == %d %s\n", v1, v2, msg);
+}
+
+static void et8ek8_check(struct et8ek8_sensor *sensor)
+{
+	/*
+	  1239 4F       # CKVAR_DIV
+	  1238 02       # CKVAR_DIV[8] CKREF_DIV
+	  123B 70       # MRCK_DIV LVDSCK_DIV
+	  123A 05       # VCO_DIV SPCK_DIV
+	  121B 63       # PIC_SIZE MONI_MODE
+	  1220 85       # H_COUNT
+	  1221 00       # H_COUNT[10:8]
+	  1222 58       # V_COUNT
+	  1223 00       # V_COUNT[12:8]
+	  121D 63       # H_SIZE H_INTERMIT
+	  125D 83       # CCP_LVDS_MODE/ _/ _/ _/ _/ CCP_COMP_MODE[2-0]
+	*/
+	struct et8ek8_reglist *r = sensor->current_reglist;
+	struct v4l2_subdev *subdev = &sensor->subdev;
+	struct i2c_client *client = v4l2_get_subdevdata(subdev);
+	int vco;
+	
+	printk("Mode validation:\n");
+
+	assert_value(client, 0x1220, (r->mode.width / 24) & 0xff);
+	assert_value(client, 0x1221, (r->mode.width / 24) >> 8);
+		
+	assert_value(client, 0x1222, (r->mode.height / 24) & 0xff);
+	assert_value(client, 0x1223, (r->mode.height / 24) >> 8);
+
+	{
+		int ckref_div = read_16(client, 0x1238) & 0xf;
+		int ckvar_div = ((read_16(client, 0x1238) & 0x80) >> 7) | (read_16(client, 0x1239) << 1);
+		int vco_div = read_16(client, 0x123A) >> 4;
+		int spck_div = read_16(client, 0x123A) & 0xf;
+		int mrck_div = read_16(client, 0x123B) >> 4;
+		int lvdsck_div = read_16(client, 0x123B) & 0xf;
+
+		vco = (r->mode.ext_clock * ckvar_div) / (ckref_div + 1);
+		printk("Vco is %d, %d %d %d\n", vco, r->mode.ext_clock, ckvar_div, ckref_div);
+		int ccp2 = vco / ((lvdsck_div + 1) * (vco_div + 1));
+		int spck = vco / ((spck_div + 1) * (vco_div + 1));
+
+		assert_eq(client, r->mode.pixel_clock, spck, "spck");
+	}
+
+	assert_eq(client, r->mode.max_exp, r->mode.height - 4, "max_exp");
+
+	assert(client, !(r->mode.sensor_window_width % r->mode.window_width), "window_width");
+	switch(r->mode.sensor_window_width / r->mode.window_width) {
+	case 1: assert_value(client, 0x121d, 0x64);
+		break;
+	case 2: assert_value(client, 0x121d, 0x63);
+		break;
+	case 3: assert_value(client, 0x121d, 0x62);
+		break;
+	default:
+		assert(client, 0, "bad window_width");
+	}
+
+	assert(client, !(r->mode.sensor_window_height % r->mode.window_height), "window_width");
+	switch(r->mode.sensor_window_height / r->mode.window_height) {
+	case 1: assert_value(client, 0x121b, 0x64);
+		break;
+	case 2: assert_value(client, 0x121b, 0x63);
+		break;
+	case 3: assert_value(client, 0x121b, 0x62);
+		break;
+	default:
+		assert(client, 0, "bad window_height");
+	}
+
+	//assert(r->mode.height * r->mode.width * fps == r->mode.pixel_clock);
+
+	switch (r->mode.bus_format) {
+	case MEDIA_BUS_FMT_SGRBG10_1X10:
+		assert_value(client, 0x125D, 0x88);
+		assert_eq(client, vco, r->mode.pixel_clock * 8, "vco_clock");
+		break;
+	case MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8:
+		assert_value(client, 0x125D, 0x83);
+		assert_eq(client, vco, r->mode.pixel_clock * 6, "vco_clock");		
+		break;
+	default:
+		assert(client, 0, "unexpected bus format");
+
+		/* There are more possibilities, see 
+		   https://github.com/maemo-foss/omap3camera-firmware/blob/master/makemodes-et8ek8.pl
+		*/
+	}
+}
+
 static int et8ek8_configure(struct et8ek8_sensor *sensor)
 {
 	struct v4l2_subdev *subdev = &sensor->subdev;
@@ -872,6 +995,8 @@ static int et8ek8_s_stream(struct v4l2_subdev *subdev, int streaming)
 	if (ret < 0)
 		return ret;
 
+	et8ek8_check(sensor);
+
 	return et8ek8_stream_on(sensor);
 }
 
diff --git a/drivers/media/i2c/et8ek8/et8ek8_mode.c b/drivers/media/i2c/et8ek8/et8ek8_mode.c
index a79882a..045d361 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_mode.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_mode.c
@@ -22,6 +22,11 @@
  * Stingray sensor mode settings for Scooby
  */
 
+/* https://github.com/maemo-foss/omap3camera-firmware/blob/master/makemodes-et8ek8.pl
+
+   /data/l/maemo/kernel-power/kernel-power-2.6.28/drivers/media/video/et8ek8-modes.h
+*/
+
 /* Mode1_poweron_Mode2_16VGA_2592x1968_12.07fps */
 static struct et8ek8_reglist mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
 /* (without the +1)
@@ -39,18 +44,20 @@ static struct et8ek8_reglist mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
  */
 	.type = ET8EK8_REGLIST_POWERON,
 	.mode = {
-		.sensor_width = 2592,
-		.sensor_height = 1968,
+	.name = "mode1_poweron_mode2_16vga_2592x1968_12_07fps",
+		
+		.sensor_width = 259,
+		.sensor_height = 196,
 		.sensor_window_origin_x = 0,
 		.sensor_window_origin_y = 0,
-		.sensor_window_width = 2592,
-		.sensor_window_height = 1968,
-		.width = 3288,
-		.height = 2016,
+		.sensor_window_width = 259,
+		.sensor_window_height = 196,
+		.width = 328,
+		.height = 201,
 		.window_origin_x = 0,
 		.window_origin_y = 0,
-		.window_width = 2592,
-		.window_height = 1968,
+		.window_width = 259,
+		.window_height = 196,
 		.pixel_clock = 80000000,
 		.ext_clock = 9600000,
 		.timeperframe = {
@@ -108,6 +115,65 @@ static struct et8ek8_reglist mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
 		{ ET8EK8_REG_8BIT, 0x1648, 0x00 },
 		{ ET8EK8_REG_8BIT, 0x113E, 0x01 },
 		{ ET8EK8_REG_8BIT, 0x113F, 0x22 },
+		/* Settings from here on seem to for the 2592x1968 mode. */
+		{ ET8EK8_REG_8BIT, 0x1239, 0x64 },
+		{ ET8EK8_REG_8BIT, 0x1238, 0x02 },
+		{ ET8EK8_REG_8BIT, 0x123B, 0x70 },
+		{ ET8EK8_REG_8BIT, 0x123A, 0x07 },
+		{ ET8EK8_REG_8BIT, 0x121B, 0x64 },
+		{ ET8EK8_REG_8BIT, 0x121D, 0x64 },
+		{ ET8EK8_REG_8BIT, 0x1221, 0x00 },
+		{ ET8EK8_REG_8BIT, 0x1220, 0x89 },
+		{ ET8EK8_REG_8BIT, 0x1223, 0x00 },
+		{ ET8EK8_REG_8BIT, 0x1222, 0x54 },
+		{ ET8EK8_REG_8BIT, 0x125D, 0x88 }, /* CCP_LVDS_MODE/  */
+		{ ET8EK8_REG_TERM, 0, 0}
+	}
+};
+
+static struct et8ek8_reglist mode2_16vga_2592x1968_12_07fps = {
+/* (without the +1)
+ * SPCK       = 80 MHz
+ * CCP2       = 640 MHz
+ * VCO        = 640 MHz
+ * VCOUNT     = 84 (2016)
+ * HCOUNT     = 137 (3288)
+ * CKREF_DIV  = 2
+ * CKVAR_DIV  = 200
+ * VCO_DIV    = 0
+ * SPCK_DIV   = 7
+ * MRCK_DIV   = 7
+ * LVDSCK_DIV = 0
+ */
+	.type = ET8EK8_REGLIST_MODE,
+	.mode = {
+	.name = "mode2_16vga_2592x1968_12_07fps",
+		
+		.sensor_width = 2592,
+		.sensor_height = 1968,
+		.sensor_window_origin_x = 0,
+		.sensor_window_origin_y = 0,
+		.sensor_window_width = 2592,
+		.sensor_window_height = 1968,
+		.width = 3288,
+		.height = 2016,
+		.window_origin_x = 0,
+		.window_origin_y = 0,
+		.window_width = 2592,
+		.window_height = 1968,
+		.pixel_clock = 80000000,
+		.ext_clock = 9600000,
+		.timeperframe = {
+			.numerator = 100,
+			.denominator = 1207
+		},
+		.max_exp = 2012,
+		/* .max_gain = 0, */
+		.bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
+		.sensitivity = 65536
+	},
+	.regs = {
+		/* Settings from here on seem to for the 2592x1968 mode. */
 		{ ET8EK8_REG_8BIT, 0x1239, 0x64 },
 		{ ET8EK8_REG_8BIT, 0x1238, 0x02 },
 		{ ET8EK8_REG_8BIT, 0x123B, 0x70 },
@@ -123,6 +189,7 @@ static struct et8ek8_reglist mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
 	}
 };
 
+
 /* Mode1_16VGA_2592x1968_13.12fps_DPCM10-8 */
 static struct et8ek8_reglist mode1_16vga_2592x1968_13_12fps_dpcm10_8 = {
 /* (without the +1)
@@ -140,6 +207,7 @@ static struct et8ek8_reglist mode1_16vga_2592x1968_13_12fps_dpcm10_8 = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode1_16vga_2592x1968_13_12fps_dpcm10_8",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -196,6 +264,7 @@ static struct et8ek8_reglist mode3_4vga_1296x984_29_99fps_dpcm10_8 = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode3_4vga_1296x984_29_99fps_dpcm10_8",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -252,6 +321,7 @@ static struct et8ek8_reglist mode4_svga_864x656_29_88fps = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode4_svga_864x656_29_88fps",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -308,6 +378,7 @@ static struct et8ek8_reglist mode5_vga_648x492_29_93fps = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode5_vga_648x492_29_93fps",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -364,6 +435,7 @@ static struct et8ek8_reglist mode2_16vga_2592x1968_3_99fps = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode2_16vga_2592x1968_3_99fps",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -398,6 +470,7 @@ static struct et8ek8_reglist mode2_16vga_2592x1968_3_99fps = {
 		{ ET8EK8_REG_8BIT, 0x1220, 0x89 },
 		{ ET8EK8_REG_8BIT, 0x1223, 0x00 },
 		{ ET8EK8_REG_8BIT, 0x1222, 0xFE },
+		{ ET8EK8_REG_8BIT, 0x125D, 0x88 }, /* CCP_LVDS_MODE/  */
 		{ ET8EK8_REG_TERM, 0, 0}
 	}
 };
@@ -419,6 +492,7 @@ static struct et8ek8_reglist mode_648x492_5fps = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode_648x492_5fps",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -475,6 +549,7 @@ static struct et8ek8_reglist mode3_4vga_1296x984_5fps = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode3_4vga_1296x984_5fps",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -531,6 +606,7 @@ static struct et8ek8_reglist mode_4vga_1296x984_25fps_dpcm10_8 = {
  */
 	.type = ET8EK8_REGLIST_MODE,
 	.mode = {
+	.name = "mode_4vga_1296x984_25fps_dpcm10_8",
 		.sensor_width = 2592,
 		.sensor_height = 1968,
 		.sensor_window_origin_x = 0,
@@ -573,15 +649,23 @@ static struct et8ek8_reglist mode_4vga_1296x984_25fps_dpcm10_8 = {
 struct et8ek8_meta_reglist meta_reglist = {
 	.version = "V14 03-June-2008",
 	.reglist = {
+		/* power on mode; strange & special */
 		{ .ptr = &mode1_poweron_mode2_16vga_2592x1968_12_07fps },
-		{ .ptr = &mode1_16vga_2592x1968_13_12fps_dpcm10_8 },
-		{ .ptr = &mode3_4vga_1296x984_29_99fps_dpcm10_8 },
-		{ .ptr = &mode4_svga_864x656_29_88fps },
-		{ .ptr = &mode5_vga_648x492_29_93fps },
-		{ .ptr = &mode2_16vga_2592x1968_3_99fps },
-		{ .ptr = &mode_648x492_5fps },
-		{ .ptr = &mode3_4vga_1296x984_5fps },
+		/* dpcm10/8 modes */
+#if 0
+		{ .ptr = &mode1_16vga_2592x1968_13_12fps_dpcm10_8 }, /* No luck */
+		{ .ptr = &mode3_4vga_1296x984_29_99fps_dpcm10_8 }, /* No luck */
 		{ .ptr = &mode_4vga_1296x984_25fps_dpcm10_8 },
+#endif
+		/* "normal" modes */
+#if 1
+		{ .ptr = &mode2_16vga_2592x1968_12_07fps }, /* My hacks. */
+		{ .ptr = &mode4_svga_864x656_29_88fps }, /* Works, AFAICT */
+		{ .ptr = &mode5_vga_648x492_29_93fps }, /* Does not seem to work ? */
+//		{ .ptr = &mode2_16vga_2592x1968_3_99fps }, /* Does not seem to work: scrolling */
+		{ .ptr = &mode_648x492_5fps }, /* Does not seem to work ? */
+		{ .ptr = &mode3_4vga_1296x984_5fps }, /* Works, AFAICT */
+#endif
 		{ .ptr = NULL }
 	}
 };
diff --git a/drivers/media/i2c/et8ek8/et8ek8_reg.h b/drivers/media/i2c/et8ek8/et8ek8_reg.h
index 07f1873..60ab305 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_reg.h
+++ b/drivers/media/i2c/et8ek8/et8ek8_reg.h
@@ -37,19 +37,21 @@ struct et8ek8_mode {
 	u16 sensor_window_height;
 
 	/* Image data coming from sensor (after scaling) */
-	u16 width;
+	u16 width;			/* u */
 	u16 height;
 	u16 window_origin_x;
 	u16 window_origin_y;
-	u16 window_width;
-	u16 window_height;
+	u16 window_width;		/* u */
+	u16 window_height;		/* u */
 
-	u32 pixel_clock;		/* in Hz */
-	u32 ext_clock;			/* in Hz */
-	struct v4l2_fract timeperframe;
-	u32 max_exp;			/* Maximum exposure value */
-	u32 bus_format;			/* MEDIA_BUS_FMT_ */
+	u32 pixel_clock;		/* u in Hz */
+	u32 ext_clock;			/* u in Hz */
+	struct v4l2_fract timeperframe; /* u */
+	u32 max_exp;			/* u Maximum exposure value */
+	u32 bus_format;			/* u MEDIA_BUS_FMT_ */
 	u32 sensitivity;		/* 16.16 fixed point */
+
+	char *name;
 };
 
 #define ET8EK8_REG_8BIT			1

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/ec784a57/attachment.sig>

^ permalink raw reply related

* [PATCH v2] ARM: dts: imx53-qsrb: Pulldown PMIC IRQ pin
From: Fabio Estevam @ 2017-04-12 21:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fabio Estevam <fabio.estevam@nxp.com>

Currently the following errors are seen:

[   14.015056] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.321093] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.411681] mc13xxx 0-0008: Failed to read IRQ status: -6
[   27.456281] mc13xxx 0-0008: Failed to read IRQ status: -6
[   30.527106] mc13xxx 0-0008: Failed to read IRQ status: -6
[   36.596900] mc13xxx 0-0008: Failed to read IRQ status: -6

Also when reading the interrupts via 'cat /proc/interrupts' the
PMIC GPIO interrupt counter does not stop increasing.

The reason for the storm of interrupts is that the PUS field of
register IOMUXC_SW_PAD_CTL_PAD_CSI0_DAT5 is currently configured as:
10 : 100k pullup

and the PMIC interrupt is being registered as IRQ_TYPE_LEVEL_HIGH type,
which is the correct type as per the MC34708 datasheet.

Use the default power on value for the IOMUX, which sets PUS field as:
00: 360k pull down

This prevents the spurious PMIC interrupts from happening.

Commit e1ffceb078c6 ("ARM: imx53: qsrb: fix PMIC interrupt level")
correctly described the irq type as IRQ_TYPE_LEVEL_HIGH, but
missed to update the IOMUX of the PMIC GPIO as pull down.

Fixes: e1ffceb078c6 ("ARM: imx53: qsrb: fix PMIC interrupt level")
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Fix typo: s/seeing/seen in the commit log

 arch/arm/boot/dts/imx53-qsrb.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx53-qsrb.dts b/arch/arm/boot/dts/imx53-qsrb.dts
index de22158..4e103a9 100644
--- a/arch/arm/boot/dts/imx53-qsrb.dts
+++ b/arch/arm/boot/dts/imx53-qsrb.dts
@@ -23,7 +23,7 @@
 	imx53-qsrb {
 		pinctrl_pmic: pmicgrp {
 			fsl,pins = <
-				MX53_PAD_CSI0_DAT5__GPIO5_23	0x1e4 /* IRQ */
+				MX53_PAD_CSI0_DAT5__GPIO5_23	0x1c4 /* IRQ */
 			>;
 		};
 	};
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-12 22:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412144109.GB6842@red-moon>

On Wed, 2017-04-12 at 15:41 +0100, Lorenzo Pieralisi wrote:
> > > At least we make the failure explicit, and the driver can take
> > > alternate (possibly sub-optimal) action if it chooses to do so.
> > 
> > The same points apply to things like pgprot_writecombine(),
> > pgprot_noncached(), pgprot_device().? Then there's also pgprot_nonposted()
> > that this series also introduces.

No. pgprot_writecombine() silently falling back to simply non-cached is
ok as we aren't "weakening" the ordering rules silently here. Something
that is correct with writecombine will also work without. It's just an
optimisation. Not correctness.

Things like noncached() must of course be honored, and I don't think
we have a case anywhere where it isn't.

My point with nopost() is that it's never ok to silently downgrade it.
Code written with the assumption that there is no posting will be
*incorrect* if posting happens. We do live with that "bug" today indeed
but once we have that accessors we might start growing more code that
relies on the specific attribute that things aren't posted and will be
wrong on all the archs providing the default implementation.

This is why I insist that pgprot_nopost() if it exists globally, should
return NULL when the semantic cannot be provided. That way there is a
clear line in the sand. If the driver choses to operate with posted
non-cached anyway, then make it an explicit driver choice.

> > If ioremap_nopost() is not possible on an architecture, then
> > pgprot_nonposted() won't be possible either - but you've made no
> > mention of that so far.

Right. It's not on most in fact.

> > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > default implementation that uses pgprot_noncached().? Maybe we should
> > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > by the architecture?

Or we *document* that mmap of IO space can result in something that is
partially non-posted.

> Yes, I was about to mention that and you are right, I should deal with
> that too unfortunately. BTW, I have not posted the drivers to make the
> review easier (ie it would add 20 more patches to an already massive
> patch series - that will be trimmed when the asm-generic include is
> removed from arches according to this discussion).
> 
> Thanks,
> Lorenzo

^ permalink raw reply

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Russell King - ARM Linux @ 2017-04-12 22:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1492036240.7236.80.camel@kernel.crashing.org>

On Thu, Apr 13, 2017 at 08:30:40AM +1000, Benjamin Herrenschmidt wrote:
> My point with nopost() is that it's never ok to silently downgrade it.
> Code written with the assumption that there is no posting will be
> *incorrect* if posting happens. We do live with that "bug" today indeed
> but once we have that accessors we might start growing more code that
> relies on the specific attribute that things aren't posted and will be
> wrong on all the archs providing the default implementation.
> 
> This is why I insist that pgprot_nopost() if it exists globally, should
> return NULL when the semantic cannot be provided.

Now you're not talking sense.  pgprot_nopost() does _not_ return a pointer.
You're talking here as if you're still talking about ioremap_nopost().
So, I think you're confused.

> > > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > > default implementation that uses pgprot_noncached().? Maybe we should
> > > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > > by the architecture?
> 
> Or we *document* that mmap of IO space can result in something that is
> partially non-posted.

Oh, so we _can_ provide an interface that has weaker semantics than it
should provided we document it.

It's insane to have different behaviours from these two interfaces, yet
you seem to have said exactly that in your reply.

It's actually worse than that - what you've just said is that it's okay
for userspace to map IO space with weaker semantics than the PCI
specification states, but it's not okay for kernel space to do that.
Especially as userspace can't know what semantics its going to end up
with, this seems to be a very strange stance to take.

I'd say that if we can't offer the no-posting behaviour that PCI
specifies, then we shouldn't be exposing IO mappings to userspace.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply

* [PATCH v4 2/2] PCI: quirks: Fix ThunderX2 dma alias handling
From: Bjorn Helgaas @ 2017-04-12 23:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412204120.GA2231@localhost>

On Wed, Apr 12, 2017 at 08:41:20PM +0000, Jayachandran C wrote:
> On Wed, Apr 12, 2017 at 02:11:38PM -0500, Bjorn Helgaas wrote:
> > On Wed, Apr 12, 2017 at 06:10:34PM +0000, Jayachandran C wrote:
> > > On Wed, Apr 12, 2017 at 11:21:18AM -0500, Bjorn Helgaas wrote:
> > > > On Tue, Apr 11, 2017 at 03:27:02PM +0000, Jayachandran C wrote:
> > > > > On Tue, Apr 11, 2017 at 08:41:25AM -0500, Bjorn Helgaas wrote:
> > > > > > [+cc Joerg]
> > > > > > 
> > > > > > On Tue, Apr 11, 2017 at 07:10:48AM +0000, Jayachandran C wrote:
> > > > > > > On Mon, Apr 10, 2017 at 08:28:47PM -0500, Bjorn Helgaas wrote:
> > > > > > > > Hi Jayachandran,
> > > > > > > > 
> > > > > > > > On Mon, Apr 03, 2017 at 01:15:04PM +0000, Jayachandran C wrote:
> > > > > > > > > The Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the PCI
> > > > > > > > > topology is slightly unusual. For a multi-node system, it looks like:
> > > > > > > > > 
> > > > > > > > > [node level PCI bridges - one per node]
> > > > > > > > >     [SoC PCI devices with MSI-X but no IOMMU]
> > > > > > > > >     [PCI-PCIe "glue" bridges - upto 14, one per real port below]
> > > > > > > > >         [PCIe real root ports associated with IOMMU and GICv3 ITS]
> > > > > > > > >             [External PCI devices connected to PCIe links]
> > > > > > > > > 
> > > > > > > > > The top two levels of bridges should have introduced aliases since they
> > > > > > > > > are PCI and PCI/PCIe bridges, but in the case of ThunderX2 they do not.
> > > > > > > > > In the case of external PCIe devices, the "real" root ports are connected
> > > > > > > > > to the SMMU and the GIC ITS, so PCI-PCIe bridge does not introduce an
> > > > > > > > > alias. The SoC PCI devices are directly connected to the GIC ITS, so the
> > > > > > > > > node level bridges do not introduce an alias either.
> > > > > > > > > 
> > > > > > > > > To handle this quirk, we mark the real PCIe root ports and node level
> > > > > > > > > PCI bridges with the flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT.  With this,
> > > > > > > > > pci_for_each_dma_alias() works correctly for external PCIe devices and
> > > > > > > > > SoC PCI devices.
> > > > > > > > > 
> > > > > > > > > For the current revision of Cavium ThunderX2, the VendorID and Device ID
> > > > > > > > > are from Broadcom Vulcan (14e4:90XX).
> > > > > > > > 
> > > > > > > > Can you supply some text here about why we want to apply this patch?
> > > > > > > > E.g., does it avoid making unnecessary IOMMU mappings, improve
> > > > > > > > performance, avoid a crash, etc?
> > > > > > > 
> > > > > > > If this is for the commit message, I hope the following is ok:
> > > > > > > 
> > > > > > > "With this change, both MSI-X and IO virtualization work correctly on
> > > > > > > Cavium ThunderX2. The GIC ITS driver gets the correct device ID to
> > > > > > > configure MSI-X, the SMMUv3 driver gets the correct Stream IDs for PCI
> > > > > > > devices, and the IOMMU groups are setup correctly."
> > > > > > 
> > > > > > This doesn't get at what the actual problem is.  I'm hoping for
> > > > > > something like "without this change, we set up an IOMMU mapping for
> > > > > > requestor ID X, but device DMA uses requestor ID Y because ...., which
> > > > > > results in an IOMMU fault"
> > > > > 
> > > > > Ok. I hope this would be better:
> > > > > 
> > > > > "Without this change, the last alias seen while traversing the PCI
> > > > > hierarchy will be used as the RID to generate the device ID for ITS
> > > > > and stream ID for SMMU. This in turn causes the MSI-X generated by the
> > > > > device to fail since the ITS expects to have translation tables based
> > > > > on the actual PCIe RID and not the (irrelevant) alias. Similarly, the
> > > > > device DMA also fails when SMMU is enabled due to incorrect value in
> > > > > SMMU translation tables"
> > > > 
> > > > This description is true, but I don't think it addresses the real
> > > > problem.  I think the real problem is that your IOMMU code doesn't
> > > > handle aliases correctly, and by ignoring these invalid aliases, we
> > > > happen to map an alias that works for the builtin devices.  But that's
> > > > only because we got lucky (those devices use a single RID and they're
> > > > not behind bridges that optionally take ownership).
> > > > 
> > > > It would make sense to me if we fixed the IOMMU code to map *all* the
> > > > aliases, which should be enough to make your devices work.  If we then
> > > > wanted to apply a patch like this on top, it would be simply an
> > > > optimization that avoids unnecessary IOMMU mappings.
> > > 
> > > The issue that the IOMMU code does not handle valid aliases is
> > > unrelated to what I am trying to fix. The quirk is to make sure
> > > that invalid aliases are not seen on ThunderX2 while doing
> > > pci_for_each_dma_alias().
> > > 
> > > The DMA and MSI-X requests leave the PCI/PCIe hierarchy at the point
> > > where the SMMU (or ITS) is attached, i.e. at the bridge marked with
> > > PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT. The quirk ensures that we don't look
> > > for aliases above that point.
> > > 
> > > The toplevel bridge is marked PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT since
> > > the on-chip devices are directly connected to the ITS (they do not
> > > use SMMU).
> > > 
> > > The top two levels of bridges are not real PCI bridges but just
> > > PCI bridge-like things that were added to tie the whole hierarchy
> > > together for configuration and enumeration. They do not handle
> > > PCI/PCIe transactions in the traditional sense.
> > > 
> > > I think my problem description is still not correct, maybe:
> > > "The SMMU (and ITS) expects to device tables to use the RID seen
> > > at the bridge they are associated with. Currently the
> > > pci_for_each_dma_alias() code traverses beyond this point and
> > > generates incorrect aliases due to the PCI and PCI/PCIe bridges
> > > above. This causes MSI-X interrupts and device DMA to fail since
> > > the SMMU and ITS tables to be setup with incorrect IDs"
> > 
> > I haven't tried to figure out the MSI-X piece of this, but let me try
> > to come up with a concrete DMA example.  Assume this topology:
> > 
> >   00:00.0 bridge to [bus 01-1e]
> >   01:0a.0 bridge to [bus 04-05]
> >   04:00.0 [14e4:9000 or 9084] bridge to [bus 05] (XLATE_ROOT)
> >   05:00.0 endpoint
> > 
> > Assume 05:00.0 generates a DMA request.  Assume the top two bridges
> > are such that pci_for_each_dma_alias() includes them as well, so it
> > iterates through 05:00.0, 01:0a.0, and 00:00.0.
> > 
> > When the driver for 05:00.0 makes a DMA mapping, the current code
> > apparently makes an IOMMU mapping for requester ID 00:00.0 because
> > that's the last alias.  Obviously this doesn't work because the IOMMU
> > at 04:00.0 will see a requester ID of 05:00.0, not 00:00.0.
> > 
> > With this quirk, we'll omit 01:0a.0 and 00:00.0, so we'll make an
> > IOMMU mapping for requester ID 05:00.0, which will work fine.  I think
> > it would *also* work fine if we made IOMMU mappings for 05:00.0,
> > 01:0a.0, and 00:0.0.  The last two are unnecessary, but probably not
> > harmful.
>  
> Ok. The last two are not harmful but still incorrect. The bridges
> 0:0.0 and 1:a.0 are outside the path with the PCI transactions takes
> (they go from 4:0.0 to the SMMU).
> 
> > Now assume 05:00 is a multi-function device that has a DMA alias
> > quirk, e.g., see quirk_dma_func0_alias().  It has another function:
> > 
> >   05:00.3 endpoint
> > 
> > DMA from 05:00.3 may use a requester ID of either 05:00.3 or 05:00.0.
> > The driver makes a DMA mapping, pci_for_each_dma_alias() iterates
> > through 05:00.3, 05:00.0, 01:0a.0, and 00:00.0, and we again map only
> > 00:00.0, which again doesn't work.
> > 
> > With this quirk, we create a single mapping for 05:00.0.  That will
> > work sometimes, but the device may also generate DMA with a requester
> > ID of 05:00.3, and that won't work.
> 
> Note that here, pci_for_each_dma_alias() works correctly with the quirk
> and incorrectly without the quirk. With the quirk, the callback function
> is involved on 05:00.3 and 05:00.0 as expected.

With the quirk, pci_for_each_dma_alias() works correctly and iterates
through 05:00.3 and 05:00.0.  But the IOMMU code only pay attention to
the *last* alias, i.e., 05:00.0.  So this does not work.

> Without the quirk the
> call back is invoked on 05:00.3, 05:00.0, 01:0a.0, and 00:00.0, which
> includes aliases that are not valid.
> 
> The idea behind the quirk is to get pci_for_each_dma_alias work correctly
> on ThunderX2, and invoke the function only on the valid aliases. With the
> quirk, the code using it - like the SMMU code - gets the correct aliases
> to process, and do not have to deal with or filter out incorrect aliases.
> 
> I don't think it would be safe to assume that all callers of
> pci_for_each_dma_alias will be ok to handle invalid aliases. Even in
> the IOMMU case, the case which calculates stream IDs is one usage, the
> usage for iommu groups also did not deal with invalid aliases correctly
> when I tried it. Then there are the MSI cases too, I had done some work
> on trying to filter out invalid aliases in the relevant code[1], but
> in my opinion, getting pci_for_each_dma_alias() do the right thing is
> the correct solution.

I agree, we should fix this and it sounds like it's more than just an
optimization.  But I don't think the quirk is a complete fix, and the
changelog needs a short sketch of this discussion to make it clear
that this happens to fix some DMA faults, but others remain because
the current IOMMU code only maps one of the valid aliases.

So I think the only thing we need to move forward on this is a revised
changelog.  I propose something like this:

  On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
  PCI topology is slightly unusual.  For a multi-node system, it looks
  like:

    00:00.0 [?? type] bridge to [bus 01-1e]
    01:0a.0 [?? type] bridge to [bus 04-05]
    04:00.0 [?? type] bridge to [bus 05] (ThunderX2 XLATE_ROOT)
    05:00.0 endpoint

  pci_for_each_dma_alias() assumes IOMMU translation is done at the
  root of the PCI hierarchy.  It generates 05:00.0 and <BB:DD.F> as
  DMA aliases for 05:00.0 because bus <BB> is a non-PCIe bus that
  doesn't carry the Requester ID.

  Because the ThunderX2 IOMMU is at 04:00.0, <BB:DD.F> is never a valid
  Requester ID.  This quirk stops alias generation at the XLATE_ROOT
  bridge so we won't generate <BB:DD.F>.

  The current IOMMU code only maps the last alias (this is a separate
  bug in itself).  Prior to this quirk, we only created IOMMU mappings
  for the invalid Requester ID <BB:DD.F>, which never matched any DMA
  transactions.
  
  With this quirk, we create IOMMU mappings for a valid Requester ID,
  which fixes devices with no aliases but leaves devices with aliases
  still broken.

I don't know the details of what type of bridges those are
(conventional PCI?  PCIe PCI-to-PCIe? etc?) and exactly what RIDs are
involved.  It'd be nice if you could fill those in.

Bjorn

^ permalink raw reply

* [PATCH] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-12 23:50 UTC (permalink / raw)
  To: linux-arm-kernel

Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
specs of the two boards:

stm32f769I discovery board:
	- Cortex-M7 core @216MHz
	- 2MB mcu internal flash
	- 512KB internal sram
	- 16MB sdram memory
	- 64MB qspi flash memory
	- 4 inch wvga LCD-TFT Display

stm32f746 discovery board:
	- Cortex-M7 core @216MHz
	- 1MB mcu internal flash
	- 320KB internal sram
	- 8MB sdram memory
	- 16MB qspi flash memory
	- 4.3 inch 480x272 LCD-TFT display

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---

Changed in v2:
- moved pin muxing from board dts file to dtsi file.

 arch/arm/boot/dts/Makefile            |  2 +
 arch/arm/boot/dts/stm32f746-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32f746.dtsi      | 12 ++++++
 arch/arm/boot/dts/stm32f769-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
 create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..a119f74 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
 dtb-$(CONFIG_ARCH_STM32)+= \
 	stm32f429-disco.dtb \
 	stm32f469-disco.dtb \
+	stm32f746-disco.dtb \
+	stm32f769-disco.dtb \
 	stm32429i-eval.dtb \
 	stm32746g-eval.dtb
 dtb-$(CONFIG_MACH_SUN4I) += \
diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
new file mode 100644
index 0000000..55afae9
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f746-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F746-DISCO board";
+	compatible = "st,stm32f746-disco", "st,stm32f746";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x800000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_pa9_pa10>;
+	pinctrl-names = "default";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..1064fb1 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,18 @@
 					bias-disable;
 				};
 			};
+			usart1_pins_pa9_pa10: usart1 at 1 {
+				pins1 {
+					pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins2 {
+					pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
+					bias-disable;
+				};
+			};
 		};
 
 		rcc: rcc at 40023800 {
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
new file mode 100644
index 0000000..166728a
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F769-DISCO board";
+	compatible = "st,stm32f769-disco", "st,stm32f7";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x1000000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_a>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
From: Vikas MANOCHA @ 2017-04-13  0:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e6592982-9c24-bfde-469b-770218bf88be@st.com>

Hi Alex,

> -----Original Message-----
> From: Alexandre TORGUE
> Sent: Tuesday, April 11, 2017 12:51 AM
> To: Vikas MANOCHA <vikas.manocha@st.com>; Patrice CHOTARD <patrice.chotard@st.com>
> Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; moderated list:ARM PORT
> <linux-arm-kernel@lists.infradead.org>; open list <linux-kernel@vger.kernel.org>; Mark Rutland <mark.rutland@arm.com>; Maxime
> Coquelin <mcoquelin.stm32@gmail.com>; Rob Herring <robh+dt@kernel.org>; Russell King <linux@armlinux.org.uk>
> Subject: Re: [PATCH] ARM: dts: stm32f7: add STM32f769I & stm32f746 discovery board support
> 
> Hi Vikas
> 
> On 04/10/2017 08:40 PM, Vikas Manocha wrote:
> > Thanks Alex,
> >
> > On 04/10/2017 12:23 AM, Alexandre Torgue wrote:
> >> Hi
> >>
> >> On 04/08/2017 03:12 AM, Vikas Manocha wrote:
> >>> Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the
> >>> major spces of the two boards:
> >>>
> >>> stm32f769I discovery board:
> >>>     - Cortex-M7 core @216MHz
> >>>     - 2MB mcu internal flash
> >>>     - 512KB internal sram
> >>>     - 16MB sdram memory
> >>>     - 64MB qspi flash memory
> >>>     - 4 inch wvga LCD-TFT Display
> >>>
> >>> stm32f746 discovery board:
> >>>     - Cortex-M7 core @216MHz
> >>>     - 1MB mcu internal flash
> >>>     - 320KB internal sram
> >>>     - 8MB sdram memory
> >>>     - 16MB qspi flash memory
> >>>     - 4.3 inch 480x272 LCD-TFT display
> >>>
> >>> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
> >>> ---
> >>>  arch/arm/boot/dts/Makefile            |   2 +
> >>>  arch/arm/boot/dts/stm32f746-disco.dts | 101 ++++++++++++++++++++++++++++++++++
> >>>  arch/arm/boot/dts/stm32f746.dtsi      |   2 +-
> >>>  arch/arm/boot/dts/stm32f769-disco.dts | 101
> >>> ++++++++++++++++++++++++++++++++++
> >>>  4 files changed, 205 insertions(+), 1 deletion(-)  create mode
> >>> 100644 arch/arm/boot/dts/stm32f746-disco.dts
> >>>  create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts
> >>>
> >>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >>> index 0118084..a119f74 100644
> >>> --- a/arch/arm/boot/dts/Makefile
> >>> +++ b/arch/arm/boot/dts/Makefile
> >>> @@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
> >>> dtb-$(CONFIG_ARCH_STM32)+= \
> >>>      stm32f429-disco.dtb \
> >>>      stm32f469-disco.dtb \
> >>> +    stm32f746-disco.dtb \
> >>> +    stm32f769-disco.dtb \
> >>>      stm32429i-eval.dtb \
> >>>      stm32746g-eval.dtb
> >>>  dtb-$(CONFIG_MACH_SUN4I) += \
> >>> diff --git a/arch/arm/boot/dts/stm32f746-disco.dts
> >>> b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> new file mode 100644
> >>> index 0000000..c0e313f
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f746-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + *  a) This file is free software; you can redistribute it and/or
> >>> + *     modify it under the terms of the GNU General Public License as
> >>> + *     published by the Free Software Foundation; either version 2 of the
> >>> + *     License, or (at your option) any later version.
> >>> + *
> >>> + *     This file is distributed in the hope that it will be useful,
> >>> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> + *     GNU General Public License for more details.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + *  b) Permission is hereby granted, free of charge, to any person
> >>> + *     obtaining a copy of this software and associated documentation
> >>> + *     files (the "Software"), to deal in the Software without
> >>> + *     restriction, including without limitation the rights to use,
> >>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + *     sell copies of the Software, and to permit persons to whom the
> >>> + *     Software is furnished to do so, subject to the following
> >>> + *     conditions:
> >>> + *
> >>> + *     The above copyright notice and this permission notice shall be
> >>> + *     included in all copies or substantial portions of the Software.
> >>> + *
> >>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + *     OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> +    model = "STMicroelectronics STM32F746-DISCO board";
> >>> +    compatible = "st,stm32f746-disco", "st,stm32f746";
> >>> +
> >>> +    chosen {
> >>> +        bootargs = "root=/dev/ram";
> >>> +        stdout-path = "serial0:115200n8";
> >>> +    };
> >>> +
> >>> +    memory {
> >>> +        reg = <0xC0000000 0x800000>;
> >>> +    };
> >>> +
> >>> +    aliases {
> >>> +        serial0 = &usart1;
> >>> +    };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> +    clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >>
> >> Pin muxing is not defined in board file. Please move it into SOC dtsi file.

Ok, I will move pin muxing in the stm32f746.dtsi file as per current implementation & send V2.

> >
> > Pin muxing used is different for different boards. e.g. usart1_rx pad is PA10 for stm32f769-disco board while it is PB7 for stm32f746-
> disco board.
> > The other possibilities for same pad (usart1_rx) is PB15. To make situation bit more complex, it is only available in f769 device.
> >
> > Putting in SOC dtsi file means having lot of combinations for different pins in separate groups.
> > e.g. only for one instance of one ip (usart1), following groups might be required at one point of time:
> >
> > usart1_pa10_pa9 {..}
> > usart1_pa10_pb14 {..}
> > usart1_pa10_pb6 {..}
> >
> > usart1_pb7_pa9 {..}
> > usart1_pb7_pb14 {..}
> > usart1_pb7_pb6 {..}
> >
> > usart1_pb15_pa9 {..}
> > usart1_pb15_pb14 {..}
> > usart1_pb15_pb6 {..}
> >
> > In case of boards based on stm32f746 device, all the above mentioned groups with pb14 & pb15 will not be available.
> > One solution (to avoid using not available groups) could be to have separate dtsi (or separate pinmux.dtsi) for different devices of
> same family like one for stm32f746 & other for stm32f769. Still it does not resolve the need to have lot of groups combinations for
> each instance of every peripheral in dtsi as mentioned above.
> 
> Yes, it is what I want to have. I did the job on for STM32F4 (F429 / F469). You could have a look on ARM Linux patchwork:
> https://patchwork.kernel.org/patch/9669433/
> 
> To sum-up the implementation:
> 
> -Pinmuxing is defined in separate files which will be included in board dts file.
> 
> -We have a common pinmuxing file + a dedicated pinmuxing file per SOC.
> 
> Example for STMF469-disco:
> stm32f4-pinctrl.dtsi --> stm32f469-pinctrl.dtsi --> stm32f469-disco.dts
> 
> stm32f4-pinctrl.dtsi contains common pinmuxing bindings between STM32F4 stm32f469-pinctrl.dtsi contains dedicated pinmuxing
> bindings for
> STM32F469 (ex: QSPI pins, gpio-ranges ...)
> 
> This implementation is under review.

Sounds good.

Cheers,
Vikas

> 
> 
> > It seems cleaner solution would be pin muxing in board dts file. Please let me know if there is some drawback of this approach. One
> point which i can think of is : duplication of pinmux groups in different board dts files.
> 
> Pinmuxing in board file is currently not my choice. In the board file we only select the group to use.
> 
> Regards
> Alex
> 
> 
> >
> > Cheers,
> > Vikas
> >
> >>> +    usart1_pins: usart1 at 0    {
> >>> +        pins1 {
> >>> +            pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> +                bias-disable;
> >>> +                drive-push-pull;
> >>> +                slew-rate = <2>;
> >>> +        };
> >>> +        pins2 {
> >>> +            pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
> >>> +            bias-disable;
> >>> +        };
> >>> +    };
> >>> +
> >>> +    qspi_pins: qspi at 0 {
> >>> +        pins {
> >>> +            pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> +                   <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> +                   <STM32F746_PD11_FUNC_QUADSPI_BK1_IO0>,
> >>> +                   <STM32F746_PD12_FUNC_QUADSPI_BK1_IO1>,
> >>> +                   <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> +                   <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> +            slew-rate = <2>;
> >>> +        };
> >>> +    };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> +    pinctrl-0 = <&usart1_pins>;
> >>> +    pinctrl-names = "default";
> >>> +    status = "okay";
> >>> +};
> >>> diff --git a/arch/arm/boot/dts/stm32f746.dtsi
> >>> b/arch/arm/boot/dts/stm32f746.dtsi
> >>> index f321ffe..826700f 100644
> >>> --- a/arch/arm/boot/dts/stm32f746.dtsi
> >>> +++ b/arch/arm/boot/dts/stm32f746.dtsi
> >>> @@ -178,7 +178,7 @@
> >>>              interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>, <42>, <62>, <76>;
> >>>          };
> >>>
> >>> -        pin-controller {
> >>> +        pinctrl: pin-controller {
> >>>              #address-cells = <1>;
> >>>              #size-cells = <1>;
> >>>              compatible = "st,stm32f746-pinctrl"; diff --git
> >>> a/arch/arm/boot/dts/stm32f769-disco.dts
> >>> b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> new file mode 100644
> >>> index 0000000..5f8558e
> >>> --- /dev/null
> >>> +++ b/arch/arm/boot/dts/stm32f769-disco.dts
> >>> @@ -0,0 +1,101 @@
> >>> +/*
> >>> + * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
> >>> + *
> >>> + * This file is dual-licensed: you can use it either under the
> >>> +terms
> >>> + * of the GPL or the X11 license, at your option. Note that this
> >>> +dual
> >>> + * licensing only applies to this file, and not this project as a
> >>> + * whole.
> >>> + *
> >>> + *  a) This file is free software; you can redistribute it and/or
> >>> + *     modify it under the terms of the GNU General Public License as
> >>> + *     published by the Free Software Foundation; either version 2 of the
> >>> + *     License, or (at your option) any later version.
> >>> + *
> >>> + *     This file is distributed in the hope that it will be useful,
> >>> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>> + *     GNU General Public License for more details.
> >>> + *
> >>> + * Or, alternatively,
> >>> + *
> >>> + *  b) Permission is hereby granted, free of charge, to any person
> >>> + *     obtaining a copy of this software and associated documentation
> >>> + *     files (the "Software"), to deal in the Software without
> >>> + *     restriction, including without limitation the rights to use,
> >>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> >>> + *     sell copies of the Software, and to permit persons to whom the
> >>> + *     Software is furnished to do so, subject to the following
> >>> + *     conditions:
> >>> + *
> >>> + *     The above copyright notice and this permission notice shall be
> >>> + *     included in all copies or substantial portions of the Software.
> >>> + *
> >>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> >>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> >>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> >>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> >>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >>> + *     OTHER DEALINGS IN THE SOFTWARE.
> >>> + */
> >>> +
> >>> +/dts-v1/;
> >>> +#include "stm32f746.dtsi"
> >>> +#include <dt-bindings/input/input.h>
> >>> +
> >>> +/ {
> >>> +    model = "STMicroelectronics STM32F769-DISCO board";
> >>> +    compatible = "st,stm32f769-disco", "st,stm32f7";
> >>> +
> >>> +    chosen {
> >>> +        bootargs = "root=/dev/ram";
> >>> +        stdout-path = "serial0:115200n8";
> >>> +    };
> >>> +
> >>> +    memory {
> >>> +        reg = <0xC0000000 0x1000000>;
> >>> +    };
> >>> +
> >>> +    aliases {
> >>> +        serial0 = &usart1;
> >>> +    };
> >>> +
> >>> +};
> >>> +
> >>> +&clk_hse {
> >>> +    clock-frequency = <25000000>;
> >>> +};
> >>> +
> >>> +&pinctrl {
> >>
> >> same.
> >>
> >>> +    usart1_pins: usart1 at 0    {
> >>> +        pins1 {
> >>> +            pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
> >>> +                bias-disable;
> >>> +                drive-push-pull;
> >>> +                slew-rate = <2>;
> >>> +        };
> >>> +        pins2 {
> >>> +            pinmux = <STM32F746_PA10_FUNC_USART1_RX>;
> >>> +            bias-disable;
> >>> +        };
> >>> +    };
> >>> +
> >>> +    qspi_pins: qspi at 0 {
> >>> +        pins {
> >>> +            pinmux = <STM32F746_PB2_FUNC_QUADSPI_CLK>,
> >>> +                   <STM32F746_PB6_FUNC_QUADSPI_BK1_NCS>,
> >>> +                   <STM32F746_PC9_FUNC_QUADSPI_BK1_IO0>,
> >>> +                   <STM32F746_PC10_FUNC_QUADSPI_BK1_IO1>,
> >>> +                   <STM32F746_PD13_FUNC_QUADSPI_BK1_IO3>,
> >>> +                   <STM32F746_PE2_FUNC_QUADSPI_BK1_IO2>;
> >>> +            slew-rate = <2>;
> >>> +        };
> >>> +    };
> >>> +};
> >>> +
> >>> +&usart1 {
> >>> +    pinctrl-0 = <&usart1_pins>;
> >>> +    pinctrl-names = "default";
> >>> +    status = "okay";
> >>> +};
> >>>
> >> .
> >>

^ permalink raw reply

* [PATCH v3] ARM: dts: stm32f7: add stm32f769I & stm32f746 discovery board support
From: Vikas Manocha @ 2017-04-13  0:27 UTC (permalink / raw)
  To: linux-arm-kernel

Stm32f769I & stm32f746 are MCUs of stm32f7 family. Here are the major
specs of the two boards:

stm32f769I discovery board:
	- Cortex-M7 core @216MHz
	- 2MB mcu internal flash
	- 512KB internal sram
	- 16MB sdram memory
	- 64MB qspi flash memory
	- 4 inch wvga LCD-TFT Display

stm32f746 discovery board:
	- Cortex-M7 core @216MHz
	- 1MB mcu internal flash
	- 320KB internal sram
	- 8MB sdram memory
	- 16MB qspi flash memory
	- 4.3 inch 480x272 LCD-TFT display

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
Changed in v3:
- fixed usart pin muxing for stm32f746 board, correct one is usart1_pins_pa9_pb7.
- Added patch version in the patch title, was missed in v2.

Changed in v2:
- moved pin muxing from board dts file to soc dtsi

 arch/arm/boot/dts/Makefile            |  2 +
 arch/arm/boot/dts/stm32f746-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/stm32f746.dtsi      | 12 ++++++
 arch/arm/boot/dts/stm32f769-disco.dts | 74 +++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+)
 create mode 100644 arch/arm/boot/dts/stm32f746-disco.dts
 create mode 100644 arch/arm/boot/dts/stm32f769-disco.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 0118084..a119f74 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -763,6 +763,8 @@ dtb-$(CONFIG_ARCH_STI) += \
 dtb-$(CONFIG_ARCH_STM32)+= \
 	stm32f429-disco.dtb \
 	stm32f469-disco.dtb \
+	stm32f746-disco.dtb \
+	stm32f769-disco.dtb \
 	stm32429i-eval.dtb \
 	stm32746g-eval.dtb
 dtb-$(CONFIG_MACH_SUN4I) += \
diff --git a/arch/arm/boot/dts/stm32f746-disco.dts b/arch/arm/boot/dts/stm32f746-disco.dts
new file mode 100644
index 0000000..52e5834
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f746-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F746-DISCO board";
+	compatible = "st,stm32f746-disco", "st,stm32f746";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x800000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_pa9_pb7>;
+	pinctrl-names = "default";
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index f321ffe..82becbb 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -287,6 +287,18 @@
 					bias-disable;
 				};
 			};
+			usart1_pins_pa9_pb7: usart1 at 1 {
+				pins1 {
+					pinmux = <STM32F746_PA9_FUNC_USART1_TX>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <0>;
+				};
+				pins2 {
+					pinmux = <STM32F746_PB7_FUNC_USART1_RX>;
+					bias-disable;
+				};
+			};
 		};
 
 		rcc: rcc at 40023800 {
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts b/arch/arm/boot/dts/stm32f769-disco.dts
new file mode 100644
index 0000000..166728a
--- /dev/null
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2017 - Vikas MANOCHA <vikas.manocha@st.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "stm32f746.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+	model = "STMicroelectronics STM32F769-DISCO board";
+	compatible = "st,stm32f769-disco", "st,stm32f7";
+
+	chosen {
+		bootargs = "root=/dev/ram";
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory {
+		reg = <0xC0000000 0x1000000>;
+	};
+
+	aliases {
+		serial0 = &usart1;
+	};
+
+};
+
+&clk_hse {
+	clock-frequency = <25000000>;
+};
+
+&usart1 {
+	pinctrl-0 = <&usart1_pins_a>;
+	pinctrl-names = "default";
+	status = "okay";
+};
-- 
1.9.1

^ permalink raw reply related

* [PATCH] [media] imx: csi: retain current field order and colorimetry setting as default
From: Steve Longerbeam @ 2017-04-13  0:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1491492354.2392.87.camel@pengutronix.de>



On 04/06/2017 08:25 AM, Philipp Zabel wrote:
> On Thu, 2017-04-06 at 16:10 +0100, Russell King - ARM Linux wrote:
>> On Thu, Apr 06, 2017 at 05:01:52PM +0200, Philipp Zabel wrote:
>>> On Thu, 2017-04-06 at 15:05 +0100, Russell King - ARM Linux wrote:
>>>> On Thu, Apr 06, 2017 at 03:55:29PM +0200, Philipp Zabel wrote:
>>>>> +
>>>>> +	/* Retain current field setting as default */
>>>>> +	if (sdformat->format.field == V4L2_FIELD_ANY)
>>>>> +		sdformat->format.field = fmt->field;
>>>>> +
>>>>> +	/* Retain current colorspace setting as default */
>>>>> +	if (sdformat->format.colorspace == V4L2_COLORSPACE_DEFAULT) {
>>>>> +		sdformat->format.colorspace = fmt->colorspace;
>>>>> +		if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT)
>>>>> +			sdformat->format.xfer_func = fmt->xfer_func;
>>>>> +		if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT)
>>>>> +			sdformat->format.ycbcr_enc = fmt->ycbcr_enc;
>>>>> +		if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT)
>>>>> +			sdformat->format.quantization = fmt->quantization;
>>>>> +	} else {
>>>>> +		if (sdformat->format.xfer_func == V4L2_XFER_FUNC_DEFAULT) {
>>>>> +			sdformat->format.xfer_func =
>>>>> +				V4L2_MAP_XFER_FUNC_DEFAULT(
>>>>> +						sdformat->format.colorspace);
>>>>> +		}
>>>>> +		if (sdformat->format.ycbcr_enc == V4L2_YCBCR_ENC_DEFAULT) {
>>>>> +			sdformat->format.ycbcr_enc =
>>>>> +				V4L2_MAP_YCBCR_ENC_DEFAULT(
>>>>> +						sdformat->format.colorspace);
>>>>> +		}
>>>>> +		if (sdformat->format.quantization == V4L2_QUANTIZATION_DEFAULT) {
>>>>> +			sdformat->format.quantization =
>>>>> +				V4L2_MAP_QUANTIZATION_DEFAULT(
>>>>> +						cc->cs != IPUV3_COLORSPACE_YUV,
>>>>> +						sdformat->format.colorspace,
>>>>> +						sdformat->format.ycbcr_enc);
>>>>> +		}
>>>>> +	}
>>>>
>>>> Would it make sense for this to be a helper function?
>>>
>>> Quite possible, the next subdev that has to set frame_interval on both
>>> pads manually because its upstream source pad doesn't suport
>>> frame_interval might want to do the same.
>>
>> Hmm.  I'm not sure I agree with this approach.  If a subdev hardware
>> does not support any modification of the colourspace or field, then
>> it should not be modifyable at the source pad - it should retain the
>> propagated settings from the sink pad.
>
> This new code is only relevant for the CSI_SINK_PAD.
>
>> I thought I had already sent a patch doing exactly that.
>
> Yes. Right above the modification there is a call to csi_try_fmt which
> will already fix up sdformat->format for the source pads. So for the
> CSI_SRC_PAD_DIRECT and CSI_SRC_PAD_IDMAC this should amount to a no-op.
>
> If might be better to move this into a separate function and only call
> it if sdformat->pad == CSI_SINK_PAD.

I've done this, I will follow with the new patch. Philipp, let me know
if this looks ok to you and I will add your sign-off.

Steve

^ permalink raw reply

* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-13  0:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170412224555.GB17774@n2100.armlinux.org.uk>

On Wed, 2017-04-12 at 23:45 +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 13, 2017 at 08:30:40AM +1000, Benjamin Herrenschmidt wrote:
> > My point with nopost() is that it's never ok to silently downgrade it.
> > Code written with the assumption that there is no posting will be
> > *incorrect* if posting happens. We do live with that "bug" today indeed
> > but once we have that accessors we might start growing more code that
> > relies on the specific attribute that things aren't posted and will be
> > wrong on all the archs providing the default implementation.
> > 
> > This is why I insist that pgprot_nopost() if it exists globally, should
> > return NULL when the semantic cannot be provided.
> 
> Now you're not talking sense.??pgprot_nopost() does _not_ return a pointer.
> You're talking here as if you're still talking about ioremap_nopost().
> So, I think you're confused.

Nah, just "typo", I meant ioremap_nopost.

> > > > Just like the proposed ioremap_nopost(), pgprot_nonposted() is given a
> > > > default implementation that uses pgprot_noncached().? Maybe we should
> > > > also make pci_remap_iospace() fail if pgprot_nonposted() is not defined
> > > > by the architecture?
> > 
> > Or we *document* that mmap of IO space can result in something that is
> > partially non-posted.
> 
> Oh, so we _can_ provide an interface that has weaker semantics than it
> should provided we document it.
> 
> It's insane to have different behaviours from these two interfaces, yet
> you seem to have said exactly that in your reply.
>
> It's actually worse than that - what you've just said is that it's okay
> for userspace to map IO space with weaker semantics than the PCI
> specification states, but it's not okay for kernel space to do that.

That is not what I'm saying. What I'm saying is that it's not ok to
provide a generic mapping attribute that silently happens to be weaker
than documented on some architectures.

The PCI part is orthogonal. How do you handle PCI in absence of that
attribute is a separate problem (which is probably a matter of just
documenting things).

BTW. Is config space also non-posted on Intel with mmconfig ? I didn't
think they could do non posted MMIO stores, but maybe I'm wrong.

> Especially as userspace can't know what semantics its going to end up
> with, this seems to be a very strange stance to take.

That's why we document that the userspace interface for *PCI* is
relaxed.

> I'd say that if we can't offer the no-posting behaviour that PCI
> specifies, then we shouldn't be exposing IO mappings to userspace.

I strongly disagree. We've been doing it for decades and it works
fine in pretty much all cases.

Note also that some platforms (including some powerpc afaik) do provide
the non-posted behaviour, simply not as a mapping attribute. Internal
fabrics aren't necessarily doing posted writes and some bridges will
hold the response for non-posted requests.

Anyway, I don't object to trying to improve compliance with the spec
on arch that have such a mapping attribute. But I do object to having
a generic mapping attribute (that isn't fundamentally a PCI thing) that
silently downgrades to something weaker.

Ben.

^ permalink raw reply

* [PATCH 1/2] phy: qcom-usb-hs: Replace the extcon API
From: Chanwoo Choi @ 2017-04-13  1:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1490675909-2533-1-git-send-email-cw00.choi@samsung.com>

Hi Kishon,

Could you please review these patches?


On 2017? 03? 28? 13:38, Chanwoo Choi wrote:
> This patch uses the resource-managed extcon API for extcon_register_notifier()
> and replaces the deprecated extcon API as following:
> - (deprecated) extcon_get_cable_state_() -> extcon_get_state()
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/phy/phy-qcom-usb-hs.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/phy/phy-qcom-usb-hs.c b/drivers/phy/phy-qcom-usb-hs.c
> index 94dfbfd739c3..f630fa553b7d 100644
> --- a/drivers/phy/phy-qcom-usb-hs.c
> +++ b/drivers/phy/phy-qcom-usb-hs.c
> @@ -156,12 +156,12 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
>  	}
>  
>  	if (uphy->vbus_edev) {
> -		state = extcon_get_cable_state_(uphy->vbus_edev, EXTCON_USB);
> +		state = extcon_get_state(uphy->vbus_edev, EXTCON_USB);
>  		/* setup initial state */
>  		qcom_usb_hs_phy_vbus_notifier(&uphy->vbus_notify, state,
>  					      uphy->vbus_edev);
> -		ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
> -				&uphy->vbus_notify);
> +		ret = devm_extcon_register_notifier(&ulpi->dev, uphy->vbus_edev,
> +				EXTCON_USB, &uphy->vbus_notify);
>  		if (ret)
>  			goto err_ulpi;
>  	}
> @@ -180,16 +180,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
>  
>  static int qcom_usb_hs_phy_power_off(struct phy *phy)
>  {
> -	int ret;
>  	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
>  
> -	if (uphy->vbus_edev) {
> -		ret = extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB,
> -						 &uphy->vbus_notify);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	regulator_disable(uphy->v3p3);
>  	regulator_disable(uphy->v1p8);
>  	clk_disable_unprepare(uphy->sleep_clk);
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox