RE: [PATCH 0/3] Renesas RZ PFC and GPIO driver

2017-01-10 Thread Chris Brandt
Hi Jacopo,

On Tuesday, January 10, 2017, Jacopo Mondi wrote:
> So, I guess what direction to take depends on whether or not we're going
> to see more hardware with a per-pin configuration that would benefit from
> this new rz-pfc driver (it seems so) and if this justify splitting sh-pfc
> in two, a group-based one for R-Car devices (and all devices there
> already) and a new pin-based one.

Well, since I'm the one with the "renesas.com" email address, I'll try to
see if I can figure out if the R-Car PFC can be changed to something else for
R-Car Gen4. Same goes for RZ/G. Sometimes the chip designers just keep doing
the same thing over and over again because they don't get any feedback from
the SW guys.


> Or maybe we can tie pin-based configuration in sh-pfc and it's me not
> seeing how to do that.

At some pointwe do need to leave the "sh-" name behind as I don't really
see much of a future for SH4 devices. Using something like pinctrl-renesas
is probably a better name. Although, anyone who's watched the semiconductor
business over the years knows that names never stay the same. (sigh)


Chris


Re: [PATCH 0/3] Renesas RZ PFC and GPIO driver

2017-01-10 Thread jacopo mondi

Hi Laurent, Chris,

On 10/01/2017 02:28, Laurent Pinchart wrote:

Hi Chris,

On Monday 09 Jan 2017 23:53:39 Chris Brandt wrote:

On Monday, January 09, 2017, Laurent Pinchart wrote:

Both the existing RZ/A model and the pinctrl model are in my opinion
improvements over the RZ/G and R-Car models. I don't care much about
whether pinctrl-single can be used, but we really need hardware
architectures that don't require those huge data tables.


I can definitely agree to that.


It's more complex than that. Both the pin-based and group-based models
have their pros and cons, and the pinctrl API is some kind of mix that
allows both options.


Here is my general question: Which of these 2 approaches are better?

A) In the DT, the user ask "enable Ethernet please" and magic happens in
   the pfc driver.

B) In the DT, the user looks up the correct pin/function assignments in
   the SoC Hardware Manual and manually spells out what they need.

R-Car looks more like A. I've been using a driver that looks more like B.

For most drivers (USB, MMC, SDHI, etc..,), I'm happy when 'magic happens'
and I don't really have to open a HW manual at all.
But, for something like setting up the PFC when someone gets a shiny new
board, making people actually open a HW manual seems acceptable to me.


From a user point of view, option A looks better to me. However, it has two
drawbacks:

- Through deciding what pin groups we make available we create a DT ABI that
will make it difficult to fix mistakes in case the groups are not fine-grained
enough.

- The data tables in C code are large, and we end up compiling many of them in
multi-platform kernel, significantly increasing the kernel size.

I would thus favour option B.



That would be saner, I agree.

And a much saner way of doing this would be, in my understanding, not 
using the group-based sh-pfc drivers used for R-Car and back it up with 
a pin-based equivalent, where to hook drivers for each specific hardware.


Looks like pinmux-single, yes, but that driver bets on the ability to 
set pin functions and configurations with a write to a single register 
while, at least for RZ/A, the same is scattered among several registers 
(I may be wrong on the single register requirement for pinctrl-single 
though)


So, I guess what direction to take depends on whether or not we're going 
to see more hardware with a per-pin configuration that would benefit 
from this new rz-pfc driver (it seems so) and if this justify splitting 
sh-pfc in two, a group-based one for R-Car devices (and all devices 
there already) and a new pin-based one.


Or maybe we can tie pin-based configuration in sh-pfc and it's me not 
seeing how to do that.


Thanks
   j



Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support

2017-01-10 Thread Wolfram Sang

> Oddly enough the error are only printed when I insert the SD card in the 
> mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
> error but the first insertion in mmc0 and boom. Only difference I can 
> see are the clock speed between mmc0 and mmc1.

Can you try this patch?

From: Wolfram Sang 
Date: Sun, 13 Nov 2016 11:10:09 +0100
Subject: [PATCH] pinctrl: pfc: r8a7795: WIP: hardcode TDSEL value

Otherwise, AC-180M won't get probed with SDR50 and EMMY-W1 has more
tuning errors with SDR104.

Signed-off-by: Wolfram Sang 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 3f58bfd676ce94..3e3f7585efe8b3 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -5416,6 +5416,9 @@ static int r8a7795_pinmux_init(struct sh_pfc *pfc)
pr_info("%s: R-Car H3 >= ES2.0\n", __func__);
// FIXME Fixup r8a7795_pinmux_info for ES2.0
}
+
+#define TDSEL 0xe60603c0
+   sh_pfc_write_reg(pfc, TDSEL, 32, 0xc3);
return 0;
 }
 
-- 
2.10.2




signature.asc
Description: PGP signature


Re: [PATCH 3/3] arm: dts: r7s72100: Add peripherals nodes

2017-01-10 Thread Geert Uytterhoeven
Hi Laurent,

On Tue, Jan 10, 2017 at 8:58 PM, Laurent Pinchart
 wrote:
> On Tuesday 10 Jan 2017 16:07:01 Geert Uytterhoeven wrote:
>> On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi wrote:
>> > From: Magnus Damm 
>> >
>> > This is a squash of several commits, adding peripherals groups
>> > configuration to r7s72100 device tree, and enabling some of them on
>> > Genmai evaluation board
>> >
>> > Signed-off-by: Jacopo Mondi 
>>
>> Thanks for the rework!
>>
>> >  arch/arm/boot/dts/r7s72100-genmai.dts |  51 
>> >  arch/arm/boot/dts/r7s72100.dtsi   | 151 +
>>
>> This path should be split in multiple parts:
>>   - Add the pfc node to r7s72100.dtsi,
>>   - Add the gpio nodes to r7s72100.dtsi,
>>   - 4 patches for r7s72100-genmai.dts, adding support for LEDs, SCIF,
>> Ethernet, and SPI.
>
> I can agree about the .dtsi/.dts split, but isn't this going a bit overboard ?

Let's find out what Simon's opinion is...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v8 6/6] mmc: sh_mobile_sdhi: Add tuning support

2017-01-10 Thread Niklas Söderlund
Hi Simon,

I started to se errors when I was testing DMAC+IPMMU patches on top of 
v4.10-rc1 on Koelsch.

[2.247490] [drm] Device feb0.display probed
[2.254407] sh_mobile_sdhi ee10.sd: Got CD GPIO
[2.259535] sh_mobile_sdhi ee10.sd: Got WP GPIO
[2.473871] sh_mobile_sdhi ee10.sd: mmc0 base at 0xee10 max clock 
rate 195 MHz
[2.484048] sh_mobile_sdhi ee14.sd: Got CD GPIO
[2.489175] sh_mobile_sdhi ee14.sd: Got WP GPIO
[2.703850] sh_mobile_sdhi ee14.sd: mmc1 base at 0xee14 max clock 
rate 97 MHz
[2.714083] sh_mobile_sdhi ee16.sd: Got CD GPIO
[2.911847] ipmmu-vmsa e674.mmu: Unhandled fault: status 0x2101 iova 
0x40002000
[2.925838] sh_mobile_sdhi ee16.sd: mmc2 base at 0xee16 max clock 
rate 97 MHz
[2.938057] ipmmu-vmsa e674.mmu: Unhandled fault: status 0x2101 iova 
0x40002000
[2.938342] asoc-simple-card sound: ak4642-hifi <-> ec50.sound mapping ok
[2.954859] mmc0: new ultra high speed SDR50 SDHC card at address 
[2.962604] mmcblk0: mmc0: SU04G 3.69 GiB 
[2.971149] input: keyboard as /devices/platform/keyboard/input/input0
[2.980044] da9063-rtc da9063-rtc: setting system clock to 2017-01-10 
20:15:43 UTC (1484079343)
[2.989814]  mmcblk0: p1
[3.106206] Micrel KSZ8041RNLI ee70.etherne:01: attached PHY driver 
[Micrel KSZ8041RNLI] (mii_bus:phy_addr=ee70.etherne:01, irq=405)

If I boot the system without a SD card inserted the warnings are not 
printed, however when I insert the SD card they are immediately printed.  
Multiple removal/insertion of a card do not trigger additional warnings, 
only at the first insertion.

Oddly enough the error are only printed when I insert the SD card in the 
mmc0 slot. I can insert/eject the card multiple times in mmc1 and no 
error but the first insertion in mmc0 and boom. Only difference I can 
see are the clock speed between mmc0 and mmc1.

[  125.681585] ipmmu-vmsa e674.mmu: Unhandled fault: status 0x2101 iova 
0x40002000
[  125.698839] ipmmu-vmsa e674.mmu: Unhandled fault: status 0x2101 iova 
0x40002000
[  125.708228] mmc0: new ultra high speed SDR50 SDHC card at address 
[  125.716394] mmcblk0: mmc0: SU04G 3.69 GiB 
[  125.737443]  mmcblk0: p1
[  305.365862] mmc0: card  removed
[  307.933518] mmc0: new ultra high speed SDR50 SDHC card at address 
[  307.941948] mmcblk0: mmc0: SU04G 3.69 GiB 
[  307.964353]  mmcblk0: p1
[  310.965794] mmc0: card  removed
[  317.335789] mmc1: new ultra high speed SDR50 SDHC card at address 
[  317.343172] mmcblk1: mmc1: SU04G 3.69 GiB 
[  317.364223]  mmcblk1: p1

Sometimes the error is reported 3 times but in most cases only 2.

I can interact fine with the card (I tried checksumming a large file and 
compared with a known good) so it's not broken. I can also interact with 
other devices using the DMAC+IPMMU without similar warnings being 
printed at all, I tested with i2c6.

If i revert this patch 06f438dd389a699d ("mmc: sh_mobile_sdhi: Add 
tuning support") the warnings go away. I have not been able to figure 
out what in the patch triggers the warnings, and I'm not sure the 
problem are with sh_mobile_sdhi. I know to little about the DMAC and 
IPMMU to rule them out as the true source. Do you have any idea what 
might cause this? I'm happy to run more tests or help out in other ways 
if I can.

To reproduce start on v4.10-rc1 and use shmobile_defconfig with a few 
additions:

CONFIG_ARM_LPAE=y
CONFIG_IPMMU_VMSA=y

And I enable IPMMU for DMAC in DT:

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 87214668d70f..d4500d79db1d 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -325,6 +325,21 @@
power-domains = < R8A7791_PD_ALWAYS_ON>;
#dma-cells = <1>;
dma-channels = <15>;
+   iommus = <_ds 0>,
+<_ds 1>,
+<_ds 2>,
+<_ds 3>,
+<_ds 4>,
+<_ds 5>,
+<_ds 6>,
+<_ds 7>,
+<_ds 8>,
+<_ds 9>,
+<_ds 10>,
+<_ds 11>,
+<_ds 12>,
+<_ds 13>,
+<_ds 14>;
};
 
dmac1: dma-controller@e672 {
@@ -356,6 +371,21 @@
power-domains = < R8A7791_PD_ALWAYS_ON>;
#dma-cells = <1>;
dma-channels = <15>;
+   iommus = <_ds 15>,
+<_ds 16>,
+<_ds 17>,
+<_ds 18>,
+<_ds 19>,
+<_ds 20>,
+<_ds 21>,
+<_ds 22>,
+<_ds 23>,
+<_ds 24>,
+   

Re: [PATCH 3/3] arm: dts: r7s72100: Add peripherals nodes

2017-01-10 Thread Laurent Pinchart
Hi Geert,

On Tuesday 10 Jan 2017 16:07:01 Geert Uytterhoeven wrote:
> On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi wrote:
> > From: Magnus Damm 
> > 
> > This is a squash of several commits, adding peripherals groups
> > configuration to r7s72100 device tree, and enabling some of them on
> > Genmai evaluation board
> > 
> > Signed-off-by: Jacopo Mondi 
> 
> Thanks for the rework!
> 
> >  arch/arm/boot/dts/r7s72100-genmai.dts |  51 
> >  arch/arm/boot/dts/r7s72100.dtsi   | 151 +
>
> This path should be split in multiple parts:
>   - Add the pfc node to r7s72100.dtsi,
>   - Add the gpio nodes to r7s72100.dtsi,
>   - 4 patches for r7s72100-genmai.dts, adding support for LEDs, SCIF,
> Ethernet, and SPI.

I can agree about the .dtsi/.dts split, but isn't this going a bit overboard ?

-- 
Regards,

Laurent Pinchart



Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs

2017-01-10 Thread Tony Lindgren
* Tony Lindgren  [170110 07:32]:
> * Geert Uytterhoeven  [170110 06:09]:
> > Hi Tony,
> > 
> > On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren  wrote:
> > > Having the pin control framework call pin controller functions
> > > before it's probe has finished is not nice as the pin controller
> > > device driver does not yet have struct pinctrl_dev handle.
> > >
> > > Let's fix this issue by adding deferred work for late init. This is
> > > needed to be able to add pinctrl generic helper functions that expect
> > > to know struct pinctrl_dev handle. Note that we now need to call
> > > create_pinctrl() directly as we don't want to add the pin controller
> > > to the list of controllers until the hogs are claimed. We also need
> > > to pass the pinctrl_dev to the device tree parser functions as they
> > > otherwise won't find the right controller at this point.
> > >
> > > Signed-off-by: Tony Lindgren 
> > 
> > I believe this patch causes a regression on r8a7740/armadillo, where the
> > pin controller is also a GPIO controller, and lcd0 needs a hog
> > (cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts):
> > 
> > -GPIO line 176 (lcd0) hogged as output/high
> > -sh-pfc e605.pfc: r8a7740_pfc handling gpio 0 -> 211
> > +gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register
> > +sh-pfc e605.pfc: failed to init GPIO chip, ignoring...
> >  sh-pfc e605.pfc: r8a7740_pfc support registered
> > 
> > Hence all drivers using GPIOs fail to initialize because their GPIOs never
> > become available.
> > 
> > Adding debug prints to the failure paths shows that the call to
> > of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER.
> > Adding a debug print to the top of gpiochip_add_data() makes the problem go
> > away, presumably because it introduces a delay that allows the delayed work
> > to kick in...
> 
> OK. What if we added also an optional pinctrl function that the pin
> controller driver could call to initialize hogs? Then the pin controller
> driver could call it during or after probe as needed. That is after
> there's a valid struct pinctrl_dev handle.
...
> We could also pass some flag if should always call pinctrl_late_init()
> directly. But that does not remove the problem of struct pinctrl_dev handle
> being uninitialized when the pin controller driver functionas are called.

Looks like we need both a flag and a way for the pin controller driver
to start things.

Below is an experimental fix to intorduce pinctrl_start() that I've
tested with pinctrl-single. Then we should probably make all pin controller
drivers call pinctrl_start() to properly fix the issue of struct pinctrl_dev
handle not being initialized before driver functions are called.

Or do you guys have any better ideas?

Regards,

Tony

8< 
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1962,6 +1962,17 @@ static void pinctrl_late_init(struct work_struct *work)
pinctrl_init_device_debugfs(pctldev);
 }
 
+int pinctrl_start(struct pinctrl_dev *pctldev)
+{
+   if (!IS_ERR(pctldev->p))
+   return -EEXIST;
+
+   pinctrl_late_init(>late_init.work);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_start);
+
 /**
  * pinctrl_register() - register a pin controller device
  * @pctldesc: descriptor for this pin controller
@@ -2035,9 +2046,14 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc 
*pctldesc,
/*
 * If the device has hogs we want the probe() function of the driver
 * to complete before we go in and hog them and add the pin controller
-* to the list of controllers. If it has no hogs, we can just complete
-* the registration immediately.
+* to the list of controllers. If the pin controller driver initializes
+* hogs, or the pin controller instance has no hogs, we can just
+* complete the registration immediately.
 */
+
+   if (pctldesc->flags & PINCTRL_DRIVER_START)
+   return pctldev;
+
if (pinctrl_dt_has_hogs(pctldev))
schedule_delayed_work(>late_init, 0);
else
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1741,6 +1741,7 @@ static int pcs_probe(struct platform_device *pdev)
pcs->desc.pmxops = _pinmux_ops;
if (PCS_HAS_PINCONF)
pcs->desc.confops = _pinconf_ops;
+   pcs->desc.flags = PINCTRL_DRIVER_START;
pcs->desc.owner = THIS_MODULE;
 
ret = pcs_allocate_pin_table(pcs);
@@ -1754,6 +1755,10 @@ static int pcs_probe(struct platform_device *pdev)
goto free;
}
 
+   ret = pinctrl_start(pcs->pctl);
+   if (ret)
+   goto free;
+
ret = pcs_add_gpio_func(np, pcs);
 

Re: [PATCH] clk: vc5: Add support for IDT VersaClock 5P49V5923B

2017-01-10 Thread Marek Vasut
On 01/10/2017 01:44 AM, Stephen Boyd wrote:
> On 01/10, Marek Vasut wrote:
>> On 01/10/2017 01:23 AM, Stephen Boyd wrote:
>>> On 01/05, Marek Vasut wrote:
 On 01/05/2017 03:13 PM, Laurent Pinchart wrote:
> Hi Marek,

 Hi!

 [...]

> +static unsigned long vc5_mux_recalc_rate(struct clk_hw *hw,
> +unsigned long parent_rate)
> +{
> +   struct vc5_driver_data *vc5 =
> +   container_of(hw, struct vc5_driver_data, clk_mux);
> +   unsigned long idiv;
> +   u8 div;
> +
> +   /* FIXME: Needs locking ? */
>>>
>>> Let's fix it then :-)
>>
>> I would like to get feedback on this one, does it ?
>
> That's a question for Mike or Stephen I believe.

 OK
>>>
>>> What's the question?
>>
>> Whether or not I need a lock around the code in vc5_mux_recalc_rate(),
>> since I am also tweaking the predivider bits there to assure the
>> (downstream) PLL supplied from the mux always gets clock in range it can
>> handle. This tweaking is mostly inspired by clk-si5351.c driver.
> 
> Don't rely on locking in the core for register locking within a
> device. That makes a dependency headache if we want to rework the
> locking scheme in the core, which we want to do eventually.
> 
> Also, please don't cause side effects during recalc_rate(). The
> callback is meant to calculate the rate of the clock, not adjust
> hardware based on what arguments are passed to it.

Got it and fixed. I'll send V3 now.

-- 
Best regards,
Marek Vasut


Re: [PATCH] watchdog: softdog: make pretimeout support a compile option

2017-01-10 Thread Guenter Roeck
On Fri, Jan 06, 2017 at 09:41:44AM +0100, Wolfram Sang wrote:
> It occured to me that the panic pretimeout governor will stall the
> softdog, because it is purely software which simply halts on panic.
> Testing governors with the softdog on the other hand is really useful.
> So, make this feature a compile time option which needs to be enabled
> explicitly. This also removes the overhead if pretimeout support is not
> used because it will now be compiled away (saving ~10% on ARM32).
> 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/watchdog/Kconfig   |  8 
>  drivers/watchdog/softdog.c | 27 +++
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index acb00b53a5207b..70726ce3d166e8 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -71,6 +71,14 @@ config SOFT_WATCHDOG
> To compile this driver as a module, choose M here: the
> module will be called softdog.
>  
> +config SOFT_WATCHDOG_PRETIMEOUT
> + bool "Software watchdog pretimeout governor support"
> + depends on SOFT_WATCHDOG && WATCHDOG_PRETIMEOUT_GOV
> + help
> +   Enable this if you want to use pretimeout governors with the software
> +   watchdog. Be aware that governors might affect the watchdog because it
> +   is purely software, e.g. the panic governor will stall it!
> +
>  config DA9052_WATCHDOG
>   tristate "Dialog DA9052 Watchdog"
>   depends on PMIC_DA9052
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index c7bdc986dca1c2..bc8b7da61d8aa7 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -74,6 +74,7 @@ static struct timer_list softdog_ticktock =
>  
>  static struct watchdog_device softdog_dev;
>  
> +#if IS_ENABLED(CONFIG_SOFT_WATCHDOG_PRETIMEOUT)
>  static void softdog_pretimeout(unsigned long data)

I would prefer __maybe_unused here ..

>  {
>   watchdog_notify_pretimeout(_dev);
> @@ -82,16 +83,23 @@ static void softdog_pretimeout(unsigned long data)
>  static struct timer_list softdog_preticktock =
>   TIMER_INITIALIZER(softdog_pretimeout, 0, 0);
>  
> +static struct timer_list *softdog_preticktock_ptr = _preticktock;
> +#else
> +static void *softdog_preticktock_ptr = NULL;
> +#endif /* CONFIG_SOFT_WATCHDOG_PRETIMEOUT */
> +
>  static int softdog_ping(struct watchdog_device *w)
>  {
>   if (!mod_timer(_ticktock, jiffies + (w->timeout * HZ)))
>   __module_get(THIS_MODULE);
>  
> - if (w->pretimeout)
> - mod_timer(_preticktock, jiffies +
> -   (w->timeout - w->pretimeout) * HZ);
> - else
> - del_timer(_preticktock);
> + if (softdog_preticktock_ptr) {

and "if (IS_ENABLED(CONFIG_SOFT_WATCHDOG_PRETIMEOUT))" here.

> + if (w->pretimeout)
> + mod_timer(softdog_preticktock_ptr, jiffies +
> +   (w->timeout - w->pretimeout) * HZ);
> + else
> + del_timer(softdog_preticktock_ptr);
> + }
>  
>   return 0;
>  }
> @@ -101,15 +109,15 @@ static int softdog_stop(struct watchdog_device *w)
>   if (del_timer(_ticktock))
>   module_put(THIS_MODULE);
>  
> - del_timer(_preticktock);
> + if (softdog_preticktock_ptr)

Is this conditional needed (assuming we get rid of softdog_preticktock_ptr) ?
Ok though if you want to use it to drop the code if not needed.

> + del_timer(softdog_preticktock_ptr);
>  
>   return 0;
>  }
>  
>  static struct watchdog_info softdog_info = {
>   .identity = "Software Watchdog",
> - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> -WDIOF_PRETIMEOUT,
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
>  };
>  
>  static const struct watchdog_ops softdog_ops = {
> @@ -134,6 +142,9 @@ static int __init softdog_init(void)
>   watchdog_set_nowayout(_dev, nowayout);
>   watchdog_stop_on_reboot(_dev);
>  
> + if (softdog_preticktock_ptr)
> + softdog_info.options |= WDIOF_PRETIMEOUT;
> +
>   ret = watchdog_register_device(_dev);
>   if (ret)
>   return ret;
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] arm64: avoid increasing DMA masks above what hardware supports

2017-01-10 Thread Robin Murphy
On 10/01/17 14:00, Nikita Yushchenko wrote:
> There are cases when device supports wide DMA addresses wider than
> device's connection supports.
> 
> In this case driver sets DMA mask based on knowledge of device
> capabilities. That must succeed to allow drivers to initialize.
> 
> However, swiotlb or iommu still need knowledge about actual device
> capabilities. To avoid breakage, actual mask must not be set wider than
> device connection allows.
> 
> Signed-off-by: Nikita Yushchenko 
> CC: Arnd Bergmann 
> CC: Robin Murphy 
> CC: Will Deacon 
> ---
>  arch/arm64/Kconfig   |  3 +++
>  arch/arm64/include/asm/device.h  |  1 +
>  arch/arm64/include/asm/dma-mapping.h |  3 +++
>  arch/arm64/mm/dma-mapping.c  | 43 
> 
>  4 files changed, 50 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1117421..afb2c08 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -216,6 +216,9 @@ config NEED_DMA_MAP_STATE
>  config NEED_SG_DMA_LENGTH
>   def_bool y
>  
> +config ARCH_HAS_DMA_SET_COHERENT_MASK
> + def_bool y
> +
>  config SMP
>   def_bool y
>  
> diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> index 243ef25..a57e7bb 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -22,6 +22,7 @@ struct dev_archdata {
>   void *iommu;/* private IOMMU data */
>  #endif
>   bool dma_coherent;
> + u64 parent_dma_mask;
>  };
>  
>  struct pdev_archdata {
> diff --git a/arch/arm64/include/asm/dma-mapping.h 
> b/arch/arm64/include/asm/dma-mapping.h
> index ccea82c..eab36d2 100644
> --- a/arch/arm64/include/asm/dma-mapping.h
> +++ b/arch/arm64/include/asm/dma-mapping.h
> @@ -51,6 +51,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
> u64 size,
>   const struct iommu_ops *iommu, bool coherent);
>  #define arch_setup_dma_ops   arch_setup_dma_ops
>  
> +#define HAVE_ARCH_DMA_SET_MASK 1
> +extern int dma_set_mask(struct device *dev, u64 dma_mask);
> +
>  #ifdef CONFIG_IOMMU_DMA
>  void arch_teardown_dma_ops(struct device *dev);
>  #define arch_teardown_dma_opsarch_teardown_dma_ops
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index e040827..7b1bb87 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -203,6 +203,37 @@ static void __dma_free(struct device *dev, size_t size,
>   __dma_free_coherent(dev, size, swiotlb_addr, dma_handle, attrs);
>  }
>  
> +int dma_set_mask(struct device *dev, u64 dma_mask)
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> + if (mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + if (ops->set_dma_mask)
> + return ops->set_dma_mask(dev, mask);
> +
> + if (!dev->dma_mask || !dma_supported(dev, mask))
> + return -EIO;
> +
> + *dev->dma_mask = mask;
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_set_mask);
> +
> +int dma_set_coherent_mask(struct device *dev, u64 mask)
> +{
> + if (mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + if (!dma_supported(dev, mask))
> + return -EIO;
> +
> + dev->coherent_dma_mask = mask;
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_set_coherent_mask);
> +
>  static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,
>unsigned long offset, size_t size,
>enum dma_data_direction dir,
> @@ -958,6 +989,18 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>   if (!dev->archdata.dma_ops)
>   dev->archdata.dma_ops = _dma_ops;
>  
> + /*
> +  * we don't yet support buses that have a non-zero mapping.
> +  *  Let's hope we won't need it
> +  */
> + WARN_ON(dma_base != 0);

I believe we now accomodate the bus remap bits on BCM2837 as a DMA
offset, so unfortunately I think this is no longer true.

> + /*
> +  * Whatever the parent bus can set. A device must not set
> +  * a DMA mask larger than this.
> +  */
> + dev->archdata.parent_dma_mask = size - 1;

This will effectively constrain *all* DMA masks to be 32-bit, since for
99% of devices we're going to see a size derived from the default mask
passed in here. I worry that that's liable to lead to performance and
stability regressions (now that the block layer can apparently generate
sufficient readahead to ovflow a typical SWIOTLB bounce buffer[1]).
Whilst DT users would be able to mitigate that by putting explicit
"dma-ranges" properties on every device node, it's less clear what we'd
do for ACPI.

I reckon the easiest way forward would be to pass in some flag to
arch_setup_dma_ops to indicate 

Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs

2017-01-10 Thread Tony Lindgren
* Geert Uytterhoeven  [170110 06:09]:
> Hi Tony,
> 
> On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren  wrote:
> > Having the pin control framework call pin controller functions
> > before it's probe has finished is not nice as the pin controller
> > device driver does not yet have struct pinctrl_dev handle.
> >
> > Let's fix this issue by adding deferred work for late init. This is
> > needed to be able to add pinctrl generic helper functions that expect
> > to know struct pinctrl_dev handle. Note that we now need to call
> > create_pinctrl() directly as we don't want to add the pin controller
> > to the list of controllers until the hogs are claimed. We also need
> > to pass the pinctrl_dev to the device tree parser functions as they
> > otherwise won't find the right controller at this point.
> >
> > Signed-off-by: Tony Lindgren 
> 
> I believe this patch causes a regression on r8a7740/armadillo, where the
> pin controller is also a GPIO controller, and lcd0 needs a hog
> (cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts):
> 
> -GPIO line 176 (lcd0) hogged as output/high
> -sh-pfc e605.pfc: r8a7740_pfc handling gpio 0 -> 211
> +gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register
> +sh-pfc e605.pfc: failed to init GPIO chip, ignoring...
>  sh-pfc e605.pfc: r8a7740_pfc support registered
> 
> Hence all drivers using GPIOs fail to initialize because their GPIOs never
> become available.
> 
> Adding debug prints to the failure paths shows that the call to
> of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER.
> Adding a debug print to the top of gpiochip_add_data() makes the problem go
> away, presumably because it introduces a delay that allows the delayed work
> to kick in...

OK. What if we added also an optional pinctrl function that the pin
controller driver could call to initialize hogs? Then the pin controller
driver could call it during or after probe as needed. That is after
there's a valid struct pinctrl_dev handle.

> Jon's fix ("pinctrl: core: Fix panic when pinctrl devices with hogs are
> unregistered") doesn't help, as it affects unregistration only.
> 
> > --- a/drivers/pinctrl/core.c
> > +++ b/drivers/pinctrl/core.c
> 
> > @@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct 
> > pinctrl_desc *pctldesc,
> > goto out_err;
> > }
> >
> > -   mutex_lock(_list_mutex);
> > -   list_add_tail(>node, _list);
> > -   mutex_unlock(_list_mutex);
> > -
> > -   pctldev->p = pinctrl_get(pctldev->dev);
> > -
> > -   if (!IS_ERR(pctldev->p)) {
> > -   pctldev->hog_default =
> > -   pinctrl_lookup_state(pctldev->p, 
> > PINCTRL_STATE_DEFAULT);
> > -   if (IS_ERR(pctldev->hog_default)) {
> > -   dev_dbg(dev, "failed to lookup the default 
> > state\n");
> > -   } else {
> > -   if (pinctrl_select_state(pctldev->p,
> > -   pctldev->hog_default))
> > -   dev_err(dev,
> > -   "failed to select default state\n");
> > -   }
> > -
> > -   pctldev->hog_sleep =
> > -   pinctrl_lookup_state(pctldev->p,
> > -   PINCTRL_STATE_SLEEP);
> > -   if (IS_ERR(pctldev->hog_sleep))
> > -   dev_dbg(dev, "failed to lookup the sleep state\n");
> > -   }
> > -
> > -   pinctrl_init_device_debugfs(pctldev);
> > +   if (pinctrl_dt_has_hogs(pctldev))
> 
> Changing the above line to "if (0 && pinctrl_dt_has_hogs(pctldev))"
> fixes the issue for me.
> 
> > +   schedule_delayed_work(>late_init, 0);
> > +   else
> > +   pinctrl_late_init(>late_init.work);
> >
> > return pctldev;

We could also pass some flag if should always call pinctrl_late_init()
directly. But that does not remove the problem of struct pinctrl_dev handle
being uninitialized when the pin controller driver functionas are called.

Regards,

Tony


Re: NVMe vs DMA addressing limitations

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 3:48:39 PM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 12:01:05PM +0100, Arnd Bergmann wrote:
> > Another workaround me might need is to limit amount of concurrent DMA
> > in the NVMe driver based on some platform quirk. The way that NVMe works,
> > it can have very large amounts of data that is concurrently mapped into
> > the device.
> 
> That's not really just NVMe - other storage and network controllers also
> can DMA map giant amounts of memory.  There are a couple aspects to it:
> 
>  - dma coherent memoery - right now NVMe doesn't use too much of it,
>but upcoming low-end NVMe controllers will soon start to require
>fairl large amounts of it for the host memory buffer feature that
>allows for DRAM-less controller designs.  As an interesting quirk
>that is memory only used by the PCIe devices, and never accessed
>by the Linux host at all.

Right, that is going to become interesting, as some platforms are
very limited with their coherent allocations.

>  - size vs number of the dynamic mapping.  We probably want the dma_ops
>specify a maximum mapping size for a given device.  As long as we
>can make progress with a few mappings swiotlb / the iommu can just
>fail mapping and the driver will propagate that to the block layer
>that throttles I/O.

Good idea.

Arnd


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 2:16:57 PM CET Robin Murphy wrote:
> On 10/01/17 13:42, Arnd Bergmann wrote:
> > On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> >> On 10/01/17 12:47, Nikita Yushchenko wrote:
>  The point here is that an IOMMU doesn't solve your issue, and the
>  IOMMU-backed DMA ops need the same treatment. In light of that, it really
>  feels to me like the DMA masks should be restricted in of_dma_configure
>  so that the parent mask is taken into account there, rather than hook
>  into each set of DMA ops to intercept set_dma_mask. We'd still need to
>  do something to stop dma_set_mask widening the mask if it was restricted
>  by of_dma_configure, but I think Robin (cc'd) was playing with that.
> >>>
> >>> What issue "IOMMU doesn't solve"?
> >>>
> >>> Issue I'm trying to address is - inconsistency within swiotlb
> >>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> >>> mask is used to decide if bounce buffers are needed or not. This
> >>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> >>> instead).
> >>
> >> The fundamental underlying problem is the "any wide mask is silently
> >> accepted" part, and that applies equally to IOMMU ops as well.
> > 
> > It's a much rarer problem for the IOMMU case though, because it only
> > impacts devices that are restricted to addressing of less than 32-bits.
> > 
> > If you have an IOMMU enabled, the dma-mapping interface does not care
> > if the device can do wider than 32 bit addressing, as it will never
> > hand out IOVAs above 0x.
> 
> I can assure you that it will - we constrain allocations to the
> intersection of the IOMMU domain aperture (normally the IOMMU's physical
> input address width) and the given device's DMA mask. If both of those
> are >32 bits then >32-bit IOVAs will fall out. For the arm64/common
> implementation I have prototyped a copy of the x86 optimisation which
> always first tries to get 32-bit IOVAs for PCI devices, but even then it
> can start returning higher addresses if the 32-bit space fills up.

Ok, got it. I have to admit that most of my knowledge about the internals
of IOMMUs is from PowerPC of a few years ago, which couldn't do this at
all. I agree that we need to do the same thing on swiotlb and iommu then.

> >> The thread Will linked to describes that equivalent version of your
> >> problem - the IOMMU gives the device 48-bit addresses which get
> >> erroneously truncated because it doesn't know that only 42 bits are
> >> actually wired up. That situation still requires the device's DMA mask
> >> to correctly describe its addressing capability just as yours does.
> > 
> > That problem should only impact virtual machines which have a guest
> > bus address space covering more than 42 bits of physical RAM, whereas
> > the problem we have with swiotlb is for the dma-mapping interface.
> > 
> I actually have a third variation of this problem involving a PCI root
> complex which *could* drive full-width (40-bit) addresses, but won't,
> due to the way its PCI<->AXI interface is programmed. That would require
> even more complicated dma-ranges handling to describe the windows of
> valid physical addresses which it *will* pass, so I'm not pressing the
> issue - let's just get the basic DMA mask case fixed first.

Can you describe this a little more? We should at least try to not
make it harder to solve the next problem while solving this one,
so I'd like to understand the exact limitation you are hitting there.

Arnd


Re: [PATCH 3/3] arm: dts: r7s72100: Add peripherals nodes

2017-01-10 Thread Geert Uytterhoeven
Hi Jacopo,

On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi  wrote:
> From: Magnus Damm 
>
> This is a squash of several commits, adding peripherals groups
> configuration to r7s72100 device tree, and enabling some of them on
> Genmai evaluation board
>
> Signed-off-by: Jacopo Mondi 

Thanks for the rework!

>  arch/arm/boot/dts/r7s72100-genmai.dts |  51 
>  arch/arm/boot/dts/r7s72100.dtsi   | 151 
> ++

This path should be split in multiple parts:
  - Add the pfc node to r7s72100.dtsi,
  - Add the gpio nodes to r7s72100.dtsi,
  - 4 patches for r7s72100-genmai.dts, adding support for LEDs, SCIF,
Ethernet, and SPI.

> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -20,6 +20,19 @@
> #size-cells = <1>;
>
> aliases {
> +   gpio0 = 
> +   gpio1 = 
> +   gpio2 = 
> +   gpio3 = 
> +   gpio4 = 
> +   gpio5 = 
> +   gpio6 = 
> +   gpio7 = 
> +   gpio8 = 
> +   gpio9 = 
> +   gpio10 = 
> +   gpio11 = 
> +   gpio12 = 

Please remove this hunk.
GPIO aliases are deprecated, and I don't think the driver uses them.

> i2c0 = 
> i2c1 = 
> i2c2 = 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 3:44:53 PM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 11:47:42AM +0100, Arnd Bergmann wrote:
> > I see that we have CONFIG_ARCH_PHYS_ADDR_T_64BIT on a couple of
> > 32-bit architectures without swiotlb (arc, arm, some mips32), and
> > there are several 64-bit architectures that do not have swiotlb
> > (alpha, parisc, s390, sparc). I believe that alpha, s390 and sparc
> > always use some form of IOMMU, but the other four apparently don't,
> > so we would need to add swiotlb support there to remove all the
> > bounce buffering in network and block layers.
> 
> mips has lots of weird swiotlb wire-up in it's board code (the swiotlb
> arch glue really needs some major cleanup..),

My reading of the MIPS code was that only the 64-bit platforms use it,
but there are a number of 32-bit platforms that have 64-bit physical
addresses and don't.

> as does arm.  Not sure about the others.

32-bit ARM doesn't actually use SWIOTLB at all, despite selecting it
in Kconfig. I think Xen uses it for its own purposes, but nothing
else does.

Most ARM platforms can't actually have RAM beyond 4GB, and the ones
that do have it tend to also come with an IOMMU, but I remember
at least BCM53xx actually needing swiotlb on some chip revisions
that are widely used and that cannot DMA to the second memory bank
from PCI (!).

Arnd


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
> On 10/01/17 12:47, Nikita Yushchenko wrote:
> >> The point here is that an IOMMU doesn't solve your issue, and the
> >> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> >> feels to me like the DMA masks should be restricted in of_dma_configure
> >> so that the parent mask is taken into account there, rather than hook
> >> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> >> do something to stop dma_set_mask widening the mask if it was restricted
> >> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> > 
> > What issue "IOMMU doesn't solve"?
> > 
> > Issue I'm trying to address is - inconsistency within swiotlb
> > dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> > mask is used to decide if bounce buffers are needed or not. This
> > inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> > instead).
> 
> The fundamental underlying problem is the "any wide mask is silently
> accepted" part, and that applies equally to IOMMU ops as well.

It's a much rarer problem for the IOMMU case though, because it only
impacts devices that are restricted to addressing of less than 32-bits.

If you have an IOMMU enabled, the dma-mapping interface does not care
if the device can do wider than 32 bit addressing, as it will never
hand out IOVAs above 0x.

> > I just can't think out what similar issue iommu can have.
> > Do you mean that in iommu case, mask also must not be set to whatever
> > wider than initial value? Why? What is the use of mask in iommu case? Is
> > there any real case when iommu can't address all memory existing in the
> > system?
> 
> There's a very subtle misunderstanding there - the DMA mask does not
> describe the memory a device can address, it describes the range of
> addresses the device is capable of generating. Yes, in the non-IOMMU
> case they are equivalent, but once you put an IOMMU in between, the
> problem is merely shifted from "what range of physical addresses can
> this device access" to "what range of IOVAs is valid to give to this
> device" - the fact that those IOVAs can map to any underlying physical
> address only obviates the need for any bouncing at the memory end; it
> doesn't remove the fact that the device has a hardware addressing
> limitation which needs to be accommodated.
> 
> The thread Will linked to describes that equivalent version of your
> problem - the IOMMU gives the device 48-bit addresses which get
> erroneously truncated because it doesn't know that only 42 bits are
> actually wired up. That situation still requires the device's DMA mask
> to correctly describe its addressing capability just as yours does.

That problem should only impact virtual machines which have a guest
bus address space covering more than 42 bits of physical RAM, whereas
the problem we have with swiotlb is for the dma-mapping interface.

> > With this direction, semantics of dma mask becomes even more
> > questionable. I'd say dma_mask is candidate for removal (or to move to
> > swiotlb's or iommu's local area)
> 
> We still need a way for drivers to communicate a device's probed
> addressing capability to SWIOTLB, so there's always going to have to be
> *some* sort of public interface. Personally, the change in semantics I'd
> like to see is to make dma_set_mask() only fail if DMA is entirely
> disallowed - in the normal case it would always succeed, but the DMA API
> implementation would be permitted to set a smaller mask than requested
> (this is effectively what the x86 IOMMU ops do already).

With swiotlb enabled, it only needs to fail if the mask does not contain
the swiotlb bounce buffer area, either because the start of RAM is outside
of the mask, or the bounce area has been allocated at the end of ZONE_DMA
and the mask is smaller than ZONE_DMA.

Arnd


Re: [PATCH 1/3] pinctrl: sh-pfc: Add r7s72100 PFC driver

2017-01-10 Thread Geert Uytterhoeven
Hi Jacopo,

On Mon, Jan 9, 2017 at 8:31 PM, Jacopo Mondi  wrote:
> From: Magnus Damm 
>
> Squash commits in Geert's renesas-driver/genmai-gpio-and-pfc branch that
> add support for r7s72100 PFC.
> This squash combines commits for Magnus' original driver and minor fixes
> to forward-port it to a more recent kernel (v4.10)
>
> Signed-off-by: Jacopo Mondi 

> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -473,6 +473,12 @@ static const struct of_device_id sh_pfc_of_table[] = {
> .data = _pinmux_info,
> },
>  #endif
> +#ifdef CONFIG_PINCTRL_PFC_R7S72100
> +   {
> +   .compatible = "renesas,pfc-r7s72100",
> +   .data = _pinmux_info,
> +   },
> +#endif
>  #ifdef CONFIG_PINCTRL_PFC_R8A73A4
> {
> .compatible = "renesas,pfc-r8a73a4",
> @@ -626,6 +632,9 @@ static int sh_pfc_probe(struct platform_device *pdev)
>  }
>
>  static const struct platform_device_id sh_pfc_id_table[] = {
> +#ifdef CONFIG_PINCTRL_PFC_R7S72100
> +   { "pfc-r7s72100", (kernel_ulong_t)_pinmux_info },
> +#endif

The above chunk should be removed, as now r7s72100 always uses DT.

>  #ifdef CONFIG_PINCTRL_PFC_SH7203
> { "pfc-sh7203", (kernel_ulong_t)_pinmux_info },
>  #endif

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Christoph Hellwig
On Tue, Jan 10, 2017 at 02:42:23PM +0100, Arnd Bergmann wrote:
> It's a much rarer problem for the IOMMU case though, because it only
> impacts devices that are restricted to addressing of less than 32-bits.
> 
> If you have an IOMMU enabled, the dma-mapping interface does not care
> if the device can do wider than 32 bit addressing, as it will never
> hand out IOVAs above 0x.

That's absolutely not the case.  IOMMUs can and do generate addresses
larger than 32-bit.  Also various platforms have modes where an IOMMU
can be used when <= 32-bit addresses are used and bypassed if full 64-bit
addressing is supported and I/O isolation is not explicitly requested.


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Christoph Hellwig
On Tue, Jan 10, 2017 at 03:47:25PM +0300, Nikita Yushchenko wrote:
> With this direction, semantics of dma mask becomes even more
> questionable. I'd say dma_mask is candidate for removal (or to move to
> swiotlb's or iommu's local area)

We need the dma mask so that the device can advertise what addresses
the device supports.  Many old devices only support 32-bit DMA addressing,
and some less common ones just 24-bit or other weird ones.


Re: NVMe vs DMA addressing limitations

2017-01-10 Thread Christoph Hellwig
On Tue, Jan 10, 2017 at 12:01:05PM +0100, Arnd Bergmann wrote:
> Another workaround me might need is to limit amount of concurrent DMA
> in the NVMe driver based on some platform quirk. The way that NVMe works,
> it can have very large amounts of data that is concurrently mapped into
> the device.

That's not really just NVMe - other storage and network controllers also
can DMA map giant amounts of memory.  There are a couple aspects to it:

 - dma coherent memoery - right now NVMe doesn't use too much of it,
   but upcoming low-end NVMe controllers will soon start to require
   fairl large amounts of it for the host memory buffer feature that
   allows for DRAM-less controller designs.  As an interesting quirk
   that is memory only used by the PCIe devices, and never accessed
   by the Linux host at all.

 - size vs number of the dynamic mapping.  We probably want the dma_ops
   specify a maximum mapping size for a given device.  As long as we
   can make progress with a few mappings swiotlb / the iommu can just
   fail mapping and the driver will propagate that to the block layer
   that throttles I/O.


Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2017-01-10 Thread Christoph Hellwig
On Tue, Jan 10, 2017 at 11:47:42AM +0100, Arnd Bergmann wrote:
> I see that we have CONFIG_ARCH_PHYS_ADDR_T_64BIT on a couple of
> 32-bit architectures without swiotlb (arc, arm, some mips32), and
> there are several 64-bit architectures that do not have swiotlb
> (alpha, parisc, s390, sparc). I believe that alpha, s390 and sparc
> always use some form of IOMMU, but the other four apparently don't,
> so we would need to add swiotlb support there to remove all the
> bounce buffering in network and block layers.

mips has lots of weird swiotlb wire-up in it's board code (the swiotlb
arch glue really needs some major cleanup..), as does arm.  Not
sure about the others.

Getting rid of highmem bouncing in the block layer will take some time
as various PIO-only drivers rely on it at the moment.  These should
all be convertable to kmap that data, but it needs a careful audit
first.  For 4.11 I'll plan to switch away from bouncing highmem by
default at least, though and maybe also convert a few PIO drivers.


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Robin Murphy
On 10/01/17 13:42, Arnd Bergmann wrote:
> On Tuesday, January 10, 2017 1:25:12 PM CET Robin Murphy wrote:
>> On 10/01/17 12:47, Nikita Yushchenko wrote:
 The point here is that an IOMMU doesn't solve your issue, and the
 IOMMU-backed DMA ops need the same treatment. In light of that, it really
 feels to me like the DMA masks should be restricted in of_dma_configure
 so that the parent mask is taken into account there, rather than hook
 into each set of DMA ops to intercept set_dma_mask. We'd still need to
 do something to stop dma_set_mask widening the mask if it was restricted
 by of_dma_configure, but I think Robin (cc'd) was playing with that.
>>>
>>> What issue "IOMMU doesn't solve"?
>>>
>>> Issue I'm trying to address is - inconsistency within swiotlb
>>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
>>> mask is used to decide if bounce buffers are needed or not. This
>>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
>>> instead).
>>
>> The fundamental underlying problem is the "any wide mask is silently
>> accepted" part, and that applies equally to IOMMU ops as well.
> 
> It's a much rarer problem for the IOMMU case though, because it only
> impacts devices that are restricted to addressing of less than 32-bits.
> 
> If you have an IOMMU enabled, the dma-mapping interface does not care
> if the device can do wider than 32 bit addressing, as it will never
> hand out IOVAs above 0x.

I can assure you that it will - we constrain allocations to the
intersection of the IOMMU domain aperture (normally the IOMMU's physical
input address width) and the given device's DMA mask. If both of those
are >32 bits then >32-bit IOVAs will fall out. For the arm64/common
implementation I have prototyped a copy of the x86 optimisation which
always first tries to get 32-bit IOVAs for PCI devices, but even then it
can start returning higher addresses if the 32-bit space fills up.

>>> I just can't think out what similar issue iommu can have.
>>> Do you mean that in iommu case, mask also must not be set to whatever
>>> wider than initial value? Why? What is the use of mask in iommu case? Is
>>> there any real case when iommu can't address all memory existing in the
>>> system?
>>
>> There's a very subtle misunderstanding there - the DMA mask does not
>> describe the memory a device can address, it describes the range of
>> addresses the device is capable of generating. Yes, in the non-IOMMU
>> case they are equivalent, but once you put an IOMMU in between, the
>> problem is merely shifted from "what range of physical addresses can
>> this device access" to "what range of IOVAs is valid to give to this
>> device" - the fact that those IOVAs can map to any underlying physical
>> address only obviates the need for any bouncing at the memory end; it
>> doesn't remove the fact that the device has a hardware addressing
>> limitation which needs to be accommodated.
>>
>> The thread Will linked to describes that equivalent version of your
>> problem - the IOMMU gives the device 48-bit addresses which get
>> erroneously truncated because it doesn't know that only 42 bits are
>> actually wired up. That situation still requires the device's DMA mask
>> to correctly describe its addressing capability just as yours does.
> 
> That problem should only impact virtual machines which have a guest
> bus address space covering more than 42 bits of physical RAM, whereas
> the problem we have with swiotlb is for the dma-mapping interface.

As above, it impacts DMA API use for anything whose addressing
capability is narrower than the IOMMU's reported input size and whose
driver is able to blindly set a too-big DMA mask. It just happens to be
the case that the stars line up on most systems, and for 32-bit devices
who keep the default DMA mask.

I actually have a third variation of this problem involving a PCI root
complex which *could* drive full-width (40-bit) addresses, but won't,
due to the way its PCI<->AXI interface is programmed. That would require
even more complicated dma-ranges handling to describe the windows of
valid physical addresses which it *will* pass, so I'm not pressing the
issue - let's just get the basic DMA mask case fixed first.

>>> With this direction, semantics of dma mask becomes even more
>>> questionable. I'd say dma_mask is candidate for removal (or to move to
>>> swiotlb's or iommu's local area)
>>
>> We still need a way for drivers to communicate a device's probed
>> addressing capability to SWIOTLB, so there's always going to have to be
>> *some* sort of public interface. Personally, the change in semantics I'd
>> like to see is to make dma_set_mask() only fail if DMA is entirely
>> disallowed - in the normal case it would always succeed, but the DMA API
>> implementation would be permitted to set a smaller mask than requested
>> (this is effectively what the x86 IOMMU ops do already).
> 
> With swiotlb enabled, it only needs 

Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs

2017-01-10 Thread Geert Uytterhoeven
Hi Tony,

On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren  wrote:
> Having the pin control framework call pin controller functions
> before it's probe has finished is not nice as the pin controller
> device driver does not yet have struct pinctrl_dev handle.
>
> Let's fix this issue by adding deferred work for late init. This is
> needed to be able to add pinctrl generic helper functions that expect
> to know struct pinctrl_dev handle. Note that we now need to call
> create_pinctrl() directly as we don't want to add the pin controller
> to the list of controllers until the hogs are claimed. We also need
> to pass the pinctrl_dev to the device tree parser functions as they
> otherwise won't find the right controller at this point.
>
> Signed-off-by: Tony Lindgren 

I believe this patch causes a regression on r8a7740/armadillo, where the
pin controller is also a GPIO controller, and lcd0 needs a hog
(cfr. arch/arm/boot/dts/r8a7740-armadillo800eva.dts):

-GPIO line 176 (lcd0) hogged as output/high
-sh-pfc e605.pfc: r8a7740_pfc handling gpio 0 -> 211
+gpiochip_add_data: GPIOs 0..211 (r8a7740_pfc) failed to register
+sh-pfc e605.pfc: failed to init GPIO chip, ignoring...
 sh-pfc e605.pfc: r8a7740_pfc support registered

Hence all drivers using GPIOs fail to initialize because their GPIOs never
become available.

Adding debug prints to the failure paths shows that the call to
of_pinctrl_get() in of_gpiochip_add_pin_range() fails with -EPROBE_DEFER.
Adding a debug print to the top of gpiochip_add_data() makes the problem go
away, presumably because it introduces a delay that allows the delayed work
to kick in...

Jon's fix ("pinctrl: core: Fix panic when pinctrl devices with hogs are
unregistered") doesn't help, as it affects unregistration only.

> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c

> @@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct 
> pinctrl_desc *pctldesc,
> goto out_err;
> }
>
> -   mutex_lock(_list_mutex);
> -   list_add_tail(>node, _list);
> -   mutex_unlock(_list_mutex);
> -
> -   pctldev->p = pinctrl_get(pctldev->dev);
> -
> -   if (!IS_ERR(pctldev->p)) {
> -   pctldev->hog_default =
> -   pinctrl_lookup_state(pctldev->p, 
> PINCTRL_STATE_DEFAULT);
> -   if (IS_ERR(pctldev->hog_default)) {
> -   dev_dbg(dev, "failed to lookup the default state\n");
> -   } else {
> -   if (pinctrl_select_state(pctldev->p,
> -   pctldev->hog_default))
> -   dev_err(dev,
> -   "failed to select default state\n");
> -   }
> -
> -   pctldev->hog_sleep =
> -   pinctrl_lookup_state(pctldev->p,
> -   PINCTRL_STATE_SLEEP);
> -   if (IS_ERR(pctldev->hog_sleep))
> -   dev_dbg(dev, "failed to lookup the sleep state\n");
> -   }
> -
> -   pinctrl_init_device_debugfs(pctldev);
> +   if (pinctrl_dt_has_hogs(pctldev))

Changing the above line to "if (0 && pinctrl_dt_has_hogs(pctldev))"
fixes the issue for me.

> +   schedule_delayed_work(>late_init, 0);
> +   else
> +   pinctrl_late_init(>late_init.work);
>
> return pctldev;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: DRM range manger test failures

2017-01-10 Thread Daniel Vetter
On Tue, Jan 10, 2017 at 12:45:04PM +, Chris Wilson wrote:
> On Tue, Jan 10, 2017 at 01:41:54PM +0100, Geert Uytterhoeven wrote:
> > Hi Chris, Laurent,
> > 
> > On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
> > manager selftests fail with:
> > 
> > drm_mm: Testing DRM range manger (struct drm_mm), with
> > random_seed=0x83a9b910 max_iterations=8192 max_prime=128
> > drm_mm: igt_sanitycheck - ok!
> > drm_mm: node has wrong color, found 0, expected 1
> > drm_mm: default insert failed, size 1 step 1
> > 
> > I have no idea what this means.
> 
> The existing code has bugs. We got as far as applying the testcases to
> demonstrate the bugs, the actual fix is still queued.

Hm, I thought I merged them all. What's missing?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Nikita Yushchenko
>> What issue "IOMMU doesn't solve"?
>>
>> Issue I'm trying to address is - inconsistency within swiotlb
>> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
>> mask is used to decide if bounce buffers are needed or not. This
>> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
>> instead).
> 
> The fundamental underlying problem is the "any wide mask is silently
> accepted" part, and that applies equally to IOMMU ops as well.

Is just posted version better?

It should cover iommu case as well.

Nikita


[PATCH] arm64: avoid increasing DMA masks above what hardware supports

2017-01-10 Thread Nikita Yushchenko
There are cases when device supports wide DMA addresses wider than
device's connection supports.

In this case driver sets DMA mask based on knowledge of device
capabilities. That must succeed to allow drivers to initialize.

However, swiotlb or iommu still need knowledge about actual device
capabilities. To avoid breakage, actual mask must not be set wider than
device connection allows.

Signed-off-by: Nikita Yushchenko 
CC: Arnd Bergmann 
CC: Robin Murphy 
CC: Will Deacon 
---
 arch/arm64/Kconfig   |  3 +++
 arch/arm64/include/asm/device.h  |  1 +
 arch/arm64/include/asm/dma-mapping.h |  3 +++
 arch/arm64/mm/dma-mapping.c  | 43 
 4 files changed, 50 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..afb2c08 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -216,6 +216,9 @@ config NEED_DMA_MAP_STATE
 config NEED_SG_DMA_LENGTH
def_bool y
 
+config ARCH_HAS_DMA_SET_COHERENT_MASK
+   def_bool y
+
 config SMP
def_bool y
 
diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 243ef25..a57e7bb 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -22,6 +22,7 @@ struct dev_archdata {
void *iommu;/* private IOMMU data */
 #endif
bool dma_coherent;
+   u64 parent_dma_mask;
 };
 
 struct pdev_archdata {
diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index ccea82c..eab36d2 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -51,6 +51,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 
size,
const struct iommu_ops *iommu, bool coherent);
 #define arch_setup_dma_ops arch_setup_dma_ops
 
+#define HAVE_ARCH_DMA_SET_MASK 1
+extern int dma_set_mask(struct device *dev, u64 dma_mask);
+
 #ifdef CONFIG_IOMMU_DMA
 void arch_teardown_dma_ops(struct device *dev);
 #define arch_teardown_dma_ops  arch_teardown_dma_ops
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index e040827..7b1bb87 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -203,6 +203,37 @@ static void __dma_free(struct device *dev, size_t size,
__dma_free_coherent(dev, size, swiotlb_addr, dma_handle, attrs);
 }
 
+int dma_set_mask(struct device *dev, u64 dma_mask)
+{
+   const struct dma_map_ops *ops = get_dma_ops(dev);
+
+   if (mask > dev->archdata.parent_dma_mask)
+   mask = dev->archdata.parent_dma_mask;
+
+   if (ops->set_dma_mask)
+   return ops->set_dma_mask(dev, mask);
+
+   if (!dev->dma_mask || !dma_supported(dev, mask))
+   return -EIO;
+
+   *dev->dma_mask = mask;
+   return 0;
+}
+EXPORT_SYMBOL(dma_set_mask);
+
+int dma_set_coherent_mask(struct device *dev, u64 mask)
+{
+   if (mask > dev->archdata.parent_dma_mask)
+   mask = dev->archdata.parent_dma_mask;
+
+   if (!dma_supported(dev, mask))
+   return -EIO;
+
+   dev->coherent_dma_mask = mask;
+   return 0;
+}
+EXPORT_SYMBOL(dma_set_coherent_mask);
+
 static dma_addr_t __swiotlb_map_page(struct device *dev, struct page *page,
 unsigned long offset, size_t size,
 enum dma_data_direction dir,
@@ -958,6 +989,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, 
u64 size,
if (!dev->archdata.dma_ops)
dev->archdata.dma_ops = _dma_ops;
 
+   /*
+* we don't yet support buses that have a non-zero mapping.
+*  Let's hope we won't need it
+*/
+   WARN_ON(dma_base != 0);
+
+   /*
+* Whatever the parent bus can set. A device must not set
+* a DMA mask larger than this.
+*/
+   dev->archdata.parent_dma_mask = size - 1;
+
dev->archdata.dma_coherent = coherent;
__iommu_setup_dma_ops(dev, dma_base, size, iommu);
 }
-- 
2.1.4



renesas-drivers-2017-01-10-v4.10-rc3

2017-01-10 Thread Geert Uytterhoeven
I have pushed renesas-drivers-2017-01-10-v4.10-rc3 to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

This tree is meant to ease development of platform support and drivers
for Renesas ARM SoCs. It is created by merging (a) the for-next branches
of various subsystem trees and (b) branches with driver code submitted
or planned for submission to maintainers into the development branch of
Simon Horman's renesas.git tree.

Today's version is based on renesas-devel-20170110-v4.10-rc3.

Included branches with driver code:
  - clk-renesas-for-v4.11
  - sh-pfc-for-v4.11
  - topic/r8a7791-pfc-adi
  - https://git.ragnatech.se/linux#for-renesas-drivers
  - topic/ipmmu-multi-arch-v6
  - topic/r8a7795-ipmmu-v2-rebased4
  - topic/r8a7796-ipmmu-v1-rebased4
  - topic/r8a7795-ipmmu-integration-v2-rebased1
  - topic/r8a7795-es2-v1-rebased7
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#vsp1/suspend-resume-race
  - git://linuxtv.org/pinchartl/media.git#drm/next/du
  - git://linuxtv.org/pinchartl/media.git#drm/next/lvds-panel
  - git://linuxtv.org/pinchartl/media.git#drm/next/lvds-encoder
  - git://linuxtv.org/pinchartl/media.git#drm/next/dw-hdmi
  - git://linuxtv.org/pinchartl/media.git#drm/du/hdmi
  - git://linuxtv.org/pinchartl/media.git#drm/du/dt/hdmi

Included fixes:
  - serial: sh-sci: Fix early deassertion of dedicated RTS
  - serial: sh-sci: Fix hang in sci_reset()

Included subsystem trees:
  - git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git#linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git#clk-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git#for-next
  - git://git.infradead.org/users/dedekind/l2-mtd-2.6.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#i2c/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git#usb-next
  - git://people.freedesktop.org/~airlied/linux#drm-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git#next
  - git://linuxtv.org/mchehab/media-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/cjb/mmc.git#mmc-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git#for-next
  - git://git.linaro.org/people/daniel.lezcano/linux.git#clockevents/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git#testing/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/djbw/dmaengine.git#next
  - git://git.infradead.org/users/vkoul/slave-dma.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next
  - git://git.armlinux.org.uk/~rmk/linux-arm.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git#for-next
  - git://git.infradead.org/users/jcooper/linux.git#irqchip/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git#for-next
  - git://git.infradead.org/battery-2.6.git#master
  - git://www.linux-watchdog.org/linux-watchdog-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git#for-next/core
  - git://anongit.freedesktop.org/drm-intel#topic/drm-misc
  - git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git#next

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Robin Murphy
On 10/01/17 12:47, Nikita Yushchenko wrote:
> Hi
> 
>> The point here is that an IOMMU doesn't solve your issue, and the
>> IOMMU-backed DMA ops need the same treatment. In light of that, it really
>> feels to me like the DMA masks should be restricted in of_dma_configure
>> so that the parent mask is taken into account there, rather than hook
>> into each set of DMA ops to intercept set_dma_mask. We'd still need to
>> do something to stop dma_set_mask widening the mask if it was restricted
>> by of_dma_configure, but I think Robin (cc'd) was playing with that.
> 
> What issue "IOMMU doesn't solve"?
> 
> Issue I'm trying to address is - inconsistency within swiotlb
> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> mask is used to decide if bounce buffers are needed or not. This
> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> instead).

The fundamental underlying problem is the "any wide mask is silently
accepted" part, and that applies equally to IOMMU ops as well.

> I just can't think out what similar issue iommu can have.
> Do you mean that in iommu case, mask also must not be set to whatever
> wider than initial value? Why? What is the use of mask in iommu case? Is
> there any real case when iommu can't address all memory existing in the
> system?

There's a very subtle misunderstanding there - the DMA mask does not
describe the memory a device can address, it describes the range of
addresses the device is capable of generating. Yes, in the non-IOMMU
case they are equivalent, but once you put an IOMMU in between, the
problem is merely shifted from "what range of physical addresses can
this device access" to "what range of IOVAs is valid to give to this
device" - the fact that those IOVAs can map to any underlying physical
address only obviates the need for any bouncing at the memory end; it
doesn't remove the fact that the device has a hardware addressing
limitation which needs to be accommodated.

The thread Will linked to describes that equivalent version of your
problem - the IOMMU gives the device 48-bit addresses which get
erroneously truncated because it doesn't know that only 42 bits are
actually wired up. That situation still requires the device's DMA mask
to correctly describe its addressing capability just as yours does.

> NVMe maintainer has just stated that they expect
> set_dma_mask(DMA_BIT_MASK(64)) to always succeed, and are going to error
> out driver probe if that call fails.  They claim that architecture must
> always be able to dma_map() whatever memory existing in the system - via
> iommu or swiotlb or whatever. Their direction is to remove bounce
> buffers from block and other layers.

I have to say I'm in full agreement with that - having subsystems do
their own bouncing is generally redundant, although there are some
edge-cases like MMC_BLOCK_BOUNCE (which is primarily about aligning and
coalescing buffers than working around DMA limitations per se).

> With this direction, semantics of dma mask becomes even more
> questionable. I'd say dma_mask is candidate for removal (or to move to
> swiotlb's or iommu's local area)

We still need a way for drivers to communicate a device's probed
addressing capability to SWIOTLB, so there's always going to have to be
*some* sort of public interface. Personally, the change in semantics I'd
like to see is to make dma_set_mask() only fail if DMA is entirely
disallowed - in the normal case it would always succeed, but the DMA API
implementation would be permitted to set a smaller mask than requested
(this is effectively what the x86 IOMMU ops do already). The significant
work that would require, though, is changing all the drivers currently
using this sort of pattern:

if (!dma_set_mask(dev, DMA_BIT_MASK(64))
/* put device into 64-bit mode */
else if (!dma_set_mask(dev, DMA_BIT_MASK(32))
/* put device into 32-bit mode */
else
/* error */

to something like this:

if (!dma_set_mask(dev, DMA_BIT_MASK(64))
/* error */
if (dma_get_mask(dev) > DMA_BIT_MASK(32))
/* put device into 64-bit mode */
else
/* put device into 32-bit mode */

Which would be a pretty major job.

Robin.


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 3:47:25 PM CET Nikita Yushchenko wrote:
> 
> > The point here is that an IOMMU doesn't solve your issue, and the
> > IOMMU-backed DMA ops need the same treatment. In light of that, it really
> > feels to me like the DMA masks should be restricted in of_dma_configure
> > so that the parent mask is taken into account there, rather than hook
> > into each set of DMA ops to intercept set_dma_mask.

of_dma_configure() sets up a 32-bit mask, which is assumed to always work
in the kernel. We can't change it to a larger mask because that would
break drivers that have to restrict devices to 32-bit.

If the bus addressing is narrower than 32 bits however, the initial mask
should probably be limited to whatever the bus supports, but that is not
the problem we are trying to solve here.

> > We'd still need to
> > do something to stop dma_set_mask widening the mask if it was restricted
> > by of_dma_configure, but I think Robin (cc'd) was playing with that.
> 
> What issue "IOMMU doesn't solve"?
> 
> Issue I'm trying to address is - inconsistency within swiotlb
> dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
> mask is used to decide if bounce buffers are needed or not. This
> inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
> instead).

It's not just an inconsistency, it's a known bug that we really
need to fix.

> I just can't think out what similar issue iommu can have.
> Do you mean that in iommu case, mask also must not be set to whatever
> wider than initial value? Why? What is the use of mask in iommu case? Is
> there any real case when iommu can't address all memory existing in the
> system?

I think the problem that Will is referring to is when the IOMMU has
a virtual address space that is wider than the DMA mask of the device:
In this case, dma_map_single() might return a dma_addr_t that is not
reachable by the device.

I'd consider that a separate bug that needs to be worked around in
the IOMMU code.

> NVMe maintainer has just stated that they expect
> set_dma_mask(DMA_BIT_MASK(64)) to always succeed, and are going to error
> out driver probe if that call fails.  They claim that architecture must
> always be able to dma_map() whatever memory existing in the system - via
> iommu or swiotlb or whatever. Their direction is to remove bounce
> buffers from block and other layers.
> 
> With this direction, semantics of dma mask becomes even more
> questionable. I'd say dma_mask is candidate for removal (or to move to
> swiotlb's or iommu's local area)

Removing dma_mask is not realistic any time soon, there are too many things
in the kernel that use it for one thing or another, so any changes here
have to be done really carefully. We definitely need the mask to support
architectures without swiotlb.

Arnd


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Nikita Yushchenko
Hi

> The point here is that an IOMMU doesn't solve your issue, and the
> IOMMU-backed DMA ops need the same treatment. In light of that, it really
> feels to me like the DMA masks should be restricted in of_dma_configure
> so that the parent mask is taken into account there, rather than hook
> into each set of DMA ops to intercept set_dma_mask. We'd still need to
> do something to stop dma_set_mask widening the mask if it was restricted
> by of_dma_configure, but I think Robin (cc'd) was playing with that.

What issue "IOMMU doesn't solve"?

Issue I'm trying to address is - inconsistency within swiotlb
dma_map_ops, where (1) any wide mask is silently accepted, but (2) then
mask is used to decide if bounce buffers are needed or not. This
inconsistency causes NVMe+R-Car cobmo not working (and breaking memory
instead).

I just can't think out what similar issue iommu can have.
Do you mean that in iommu case, mask also must not be set to whatever
wider than initial value? Why? What is the use of mask in iommu case? Is
there any real case when iommu can't address all memory existing in the
system?

NVMe maintainer has just stated that they expect
set_dma_mask(DMA_BIT_MASK(64)) to always succeed, and are going to error
out driver probe if that call fails.  They claim that architecture must
always be able to dma_map() whatever memory existing in the system - via
iommu or swiotlb or whatever. Their direction is to remove bounce
buffers from block and other layers.

With this direction, semantics of dma mask becomes even more
questionable. I'd say dma_mask is candidate for removal (or to move to
swiotlb's or iommu's local area)

Nikita


Re: DRM range manger test failures

2017-01-10 Thread Chris Wilson
On Tue, Jan 10, 2017 at 01:41:54PM +0100, Geert Uytterhoeven wrote:
> Hi Chris, Laurent,
> 
> On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
> manager selftests fail with:
> 
> drm_mm: Testing DRM range manger (struct drm_mm), with
> random_seed=0x83a9b910 max_iterations=8192 max_prime=128
> drm_mm: igt_sanitycheck - ok!
> drm_mm: node has wrong color, found 0, expected 1
> drm_mm: default insert failed, size 1 step 1
> 
> I have no idea what this means.

The existing code has bugs. We got as far as applying the testcases to
demonstrate the bugs, the actual fix is still queued.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


DRM range manger test failures

2017-01-10 Thread Geert Uytterhoeven
Hi Chris, Laurent,

On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
manager selftests fail with:

drm_mm: Testing DRM range manger (struct drm_mm), with
random_seed=0x83a9b910 max_iterations=8192 max_prime=128
drm_mm: igt_sanitycheck - ok!
drm_mm: node has wrong color, found 0, expected 1
drm_mm: default insert failed, size 1 step 1

I have no idea what this means.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2] arm64: do not set dma masks that device connection can't handle

2017-01-10 Thread Will Deacon
On Mon, Jan 09, 2017 at 10:30:02AM +0300, Nikita Yushchenko wrote:
> It is possible that device is capable of 64-bit DMA addresses, and
> device driver tries to set wide DMA mask, but bridge or bus used to
> connect device to the system can't handle wide addresses.
> 
> With swiotlb, memory above 4G still can be used by drivers for streaming
> DMA, but *dev->mask and dev->dma_coherent_mask must still keep values
> that hardware handles physically.
> 
> This patch enforces that. Based on original version by
> Arnd Bergmann , extended with coherent mask hadnling.
> 
> Signed-off-by: Nikita Yushchenko 
> CC: Arnd Bergmann 
> ---
> Changes since v1:
> - fixed issues noted by Sergei Shtylyov 
>   - save mask, not size
>   - remove doube empty line
> 
>  arch/arm64/Kconfig  |  3 +++
>  arch/arm64/include/asm/device.h |  1 +
>  arch/arm64/mm/dma-mapping.c | 51 
> +
>  3 files changed, 55 insertions(+)

I still don't think this patch is general enough. The problem you're seeing
with swiotlb seems to be exactly the same problem reported by Feng Kan over
at:

  
http://lkml.kernel.org/r/cal85gma_sscwm80tkdkzqee+s1bewzdevdki1kpkmutdrms...@mail.gmail.com

[read on; it was initially thought to be a hardware erratum, but it's
 actually the inability to restrict the DMA mask of the endpoint that's
 the problem]

The point here is that an IOMMU doesn't solve your issue, and the
IOMMU-backed DMA ops need the same treatment. In light of that, it really
feels to me like the DMA masks should be restricted in of_dma_configure
so that the parent mask is taken into account there, rather than hook
into each set of DMA ops to intercept set_dma_mask. We'd still need to
do something to stop dma_set_mask widening the mask if it was restricted
by of_dma_configure, but I think Robin (cc'd) was playing with that.

Will


Re: NVMe vs DMA addressing limitations

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 10:31:47 AM CET Nikita Yushchenko wrote:
> Christoph, thanks for clear input.
> 
> Arnd, I think that given this discussion, best short-term solution is
> indeed the patch I've submitted yesterday. That is, your version +
> coherent mask support.  With that, set_dma_mask(DMA_BIT_MASK(64)) will
> succeed and hardware with work with swiotlb.

Ok, good.

> Possible next step is to teach swiotlb to dynamically allocate bounce
> buffers within entire arm64's ZONE_DMA.

That seems reasonable, yes. We probably have to do both, as there are
cases where a device has dma_mask smaller than ZONE_DMA but the swiotlb
bounce area is low enough to work anyway.

Another workaround me might need is to limit amount of concurrent DMA
in the NVMe driver based on some platform quirk. The way that NVMe works,
it can have very large amounts of data that is concurrently mapped into
the device. SWIOTLB is one case where this currently fails, but another
example would be old PowerPC servers that have a 256MB window of virtual
I/O addresses per VM guest in their IOMMU. Those will likely fail the same
way that your does.

> Also there is some hope that R-Car *can* iommu-translate addresses that
> PCIe module issues to system bus.  Although previous attempts to make
> that working failed. Additional research is needed here.

Does this IOMMU support remapping data within a virtual machine? I believe
there are some that only do one of the two -- either you can have guest
machines with DMA access to their low memory, or you can remap data on
the fly in the host.

Arnd


Re: NVMe vs DMA addressing limitations

2017-01-10 Thread Arnd Bergmann
On Tuesday, January 10, 2017 8:07:20 AM CET Christoph Hellwig wrote:
> On Tue, Jan 10, 2017 at 09:47:21AM +0300, Nikita Yushchenko wrote:
> > I'm now working with HW that:
> > - is now way "low end" or "obsolete", it has 4G of RAM and 8 CPU cores,
> > and is being manufactured and developed,
> > - has 75% of it's RAM located beyond first 4G of address space,
> > - can't physically handle incoming PCIe transactions addressed to memory
> > beyond 4G.
> 
> It might not be low end or obselete, but it's absolutely braindead.
> Your I/O performance will suffer badly for the life of the platform
> because someone tries to save 2 cents, and there is not much we can do
> about it.

Unfortunately it is a common problem for arm64 chips that were designed
by taking a 32-bit SoC and replacing the CPU core. The swiotlb is the
right workaround for this, and I think we all agree that we should
just make it work correctly.

Arnd


Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask

2017-01-10 Thread Arnd Bergmann
On Monday, January 9, 2017 9:57:46 PM CET Christoph Hellwig wrote:
> > - architecture should stop breaking 64-bit DMA when driver attempts to
> > set 64-bit dma mask,
> > 
> > - NVMe should issue proper blk_queue_bounce_limit() call based on what
> > is actually set mask,
> 
> Or even better remove the call to dma_set_mask_and_coherent with
> DMA_BIT_MASK(32).  NVMe is designed around having proper 64-bit DMA
> addressing, there is not point in trying to pretent it works without that

Agreed, let's just fail the probe() if DMA_BIT_MASK(64) fails, and
have swiotlb work around machines that for some reason need bounce
buffers.

> > - and blk_queue_bounce_limit() should also be fixed to actually set
> > 0x limit, instead of replacing it with (max_low_pfn <<
> > PAGE_SHIFT) as it does now.
> 
> We need to kill off BLK_BOUNCE_HIGH, it just doesn't make sense to
> mix the highmem aspect with the addressing limits.  In fact the whole
> block bouncing scheme doesn't make much sense at all these days, we
> should rely on swiotlb instead.

If we do this, we should probably have another look at the respective
NETIF_F_HIGHDMA support in the network stack, which does the same thing
and mixes up highmem on 32-bit architectures with the DMA address limit.
(side note: there are actually cases in which you have a 31-bit DMA
mask but 3 GB of lowmem using CONFIG_VMSPLIT_1G, so BLK_BOUNCE_HIGH
and !NETIF_F_HIGHDMA are both missing the limit, causing data corruption
without swiotlb).

Before we rely too much on swiotlb, we may also need to consider which
architectures today rely on bouncing in blk and network.

I see that we have CONFIG_ARCH_PHYS_ADDR_T_64BIT on a couple of
32-bit architectures without swiotlb (arc, arm, some mips32), and
there are several 64-bit architectures that do not have swiotlb
(alpha, parisc, s390, sparc). I believe that alpha, s390 and sparc
always use some form of IOMMU, but the other four apparently don't,
so we would need to add swiotlb support there to remove all the
bounce buffering in network and block layers.

Arnd


Re: [PATCH v4 0/4] thermal: add driver for R-Car Gen3

2017-01-10 Thread Geert Uytterhoeven
Hi Wolfram,

On Thu, Dec 1, 2016 at 11:04 PM, Wolfram Sang
 wrote:
> Here is the next series for adding thermal support to Renesas R-Car Gen3 SoCs.
> All changes from last revision went into patch 2/4, so please check the
> changelog there. This series has again been tested on Salvator-X boards with
> H3 and M3-W SoCs. The updated branch can be found here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
> renesas/thermal-on-4.10-next

Dropped from renesas-drivers, as Niklas provided a branch with the
reworked version.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH 0/6] R-Car DU: Fix IOMMU operation when connected to VSP

2017-01-10 Thread Geert Uytterhoeven
Hi Laurent,

On Fri, Aug 19, 2016 at 10:39 AM, Laurent Pinchart
 wrote:
> This patch series fixes the rcar-du-drm driver to support VSP plane sources
> with an IOMMU. It is available for convenience at
>
> git://linuxtv.org/pinchartl/media.git iommu/devel/du

Dropped from renesas-drivers, as this branch is based on a very old tree
(v4.8-rc2), and many (but not all!) commits have found their way upstream.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [GIT PULL FOR renesas-drivers] VSP Writeback prototype

2017-01-10 Thread Geert Uytterhoeven
On Mon, Nov 14, 2016 at 9:17 PM, Kieran Bingham
 wrote:
> The following changes since commit 8bc55e5947b58dee3c8b126d7c303991ae0db130:
>
>   tty: serial_core: Fix serial console crash on port shutdown (2016-10-25 
> 13:53:49 +0200)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
> tags/vsp1/writeback/v2

Dropped from renesas-drivers, as this branch is based on a very old tree
(v4.9-rc2), and many (but not all!) commits have found their way upstream.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [GIT PULL FOR renesas-drivers] VSP Partition algorithm improvements

2017-01-10 Thread Geert Uytterhoeven
Hi Kieran,

On Mon, Nov 14, 2016 at 9:17 PM, Kieran Bingham
 wrote:
> The following changes since commit 8bc55e5947b58dee3c8b126d7c303991ae0db130:
>
>   tty: serial_core: Fix serial console crash on port shutdown (2016-10-25 
> 13:53:49 +0200)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git 
> tags/vsp1/pa-improvements/v1

Dropped from renesas-drivers, as this branch is based on a very old tree
(v4.9-rc2), and many (but not all!) commits have found their way upstream.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2 0/5] r8a7796 I2C integration

2017-01-10 Thread Geert Uytterhoeven
Hi Uli,

On Thu, Nov 3, 2016 at 5:06 PM, Ulrich Hecht
 wrote:
> On Mon, Oct 31, 2016 at 12:46 PM, Wolfram Sang  wrote:
>>
>>> This revision is based on renesas-devel-20161024-v4.9-rc2.  It adds the
>>> Reviewed-bys and the entries for SYS-DMAC2 in the applicable I2C nodes.  I
>>> have also made sure it works with 64-bit memory enabled.
>>
>> I'd like to test, but: can you tell me which branches are needed to test
>> DMA? Or is there a complete branch available somewhere? Or could you
>> update the branch of this feature which is pulled in by renesas-drivers?
>
> Thank you. Please check branch r8a7796-dmac-v2 at
> https://github.com/uli/kernel.git, that should contain everything you
> need.

Dropped from renesas-drivers, as this branch is based on a very old tree
(v4.9-rc2), and many (not all!) commits have found their way upstream.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


RE: [PATCH v2 6/7] dt-bindings: media: Add Renesas R-Car DRIF binding

2017-01-10 Thread Ramesh Shanmugasundaram
Hi Laurent,

> > >>> On Wednesday 21 Dec 2016 08:10:37 Ramesh Shanmugasundaram wrote:
> >  Add binding documentation for Renesas R-Car Digital Radio
> >  Interface
> >  (DRIF) controller.
> > 
> >  Signed-off-by: Ramesh Shanmugasundaram
> >   ---
> > 
> >   .../devicetree/bindings/media/renesas,drif.txt | 202
> +
> >   1 file changed, 202 insertions(+)  create mode 100644
> > 
> >  Documentation/devicetree/bindings/media/renesas,drif.txt
> > 
> >  diff --git
> >  a/Documentation/devicetree/bindings/media/renesas,drif.txt
> >  b/Documentation/devicetree/bindings/media/renesas,drif.txt new
> >  file mode 100644 index 000..1f3feaf
> >  --- /dev/null
> >  +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> > 
> >  +Optional properties of an internal channel when:
> >  + - It is the only enabled channel of the bond (or)
> >  + - If it acts as primary among enabled bonds
> >  +
> >  +- renesas,syncmd   : sync mode
> >  +  0 (Frame start sync pulse mode. 1-bit
> >  +width
> >  pulse
> >  + indicates start of a frame)
> >  +  1 (L/R sync or I2S mode) (default)
> >  +- renesas,lsb-first: empty property indicates lsb bit is
> received
> >  first.
> >  +  When not defined msb bit is received first
> >  +(default)
> >  +- renesas,syncac-active: Indicates sync signal polarity, 0/1 for
> >  low/high
> >
> > Shouldn't this be 'renesas,sync-active' instead of syncac-active?
> >
> > I'm not sure if syncac is intended or if it is a typo.
> >
> >  +  respectively. The default is 1 (active high)
> >  +- renesas,dtdl : delay between sync signal and start of
> >  reception.
> >  +  The possible values are represented in 0.5
> clock
> >  +  cycle units and the range is 0 to 4. The
> default
> >  +  value is 2 (i.e.) 1 clock cycle delay.
> >  +- renesas,syncdl   : delay between end of reception and sync
> >  signal edge.
> >  +  The possible values are represented in 0.5
> clock
> >  +  cycle units and the range is 0 to 4 & 6.
> >  + The
> >  default
> >  +  value is 0 (i.e.) no delay.
> > >>>
> > >>> Most of these properties are pretty similar to the video bus
> > >>> properties defined at the endpoint level in
> > >>> Documentation/devicetree/bindings/media/video-interfaces.txt. I
> > >>> believe it would make sense to use OF graph and try to standardize
> > >>> these properties similarly.
> >
> > Other than sync-active, is there really anything else that is similar?
> > And even the sync-active isn't a good fit since here there is only one
> > sync signal instead of two for video (h and vsync).
> 
> That's why I said similar, not identical :-) My point is that, if we
> consider that we could connect multiple sources to the DRIF, using OF
> graph would make sense, and the above properties should then be defined
> per endpoint.

Thanks for the clarifications. I have some questions.

- Assuming two devices are interfaced with DRIF and they are represented using 
two endpoints, the control signal related properties of DRIF might still need 
to be same for both endpoints? For e.g. syncac-active cannot be different in 
both endpoints?

- I suppose "lsb-first", "dtdl" & "syncdl" may be defined per endpoint. 
However, h/w manual says same register values needs to be programmed for both 
the internal channels of a channel. Same with "syncmd" property.

We could still define them as per endpoint property with a note that they need 
to be same. But I am not sure if that is what you intended?

 If we define them per endpoint we should then also try
> standardize the ones that are not really Renesas-specific (that's at least
> syncac-active).

OK. I will call it "sync-active".

 For the syncmd and lsb-first properties, it could also
> make sense to query them from the connected subdev at runtime, as they're
> similar in purpose to formats and media bus configuration (struct
> v4l2_mbus_config).

May I know in bit more detail about what you had in mind? Please correct me if 
my understanding is wrong here but when I looked at the code

1) mbus_config is part of subdev_video_ops only. I assume we don't want to 
support this as part of tuner subdev. The next closest is pad_ops with "struct 
v4l2_mbus_framefmt" but it is fully video specific config unless I come up with 
new MEDIA_BUS_FMT_ in media-bus-format.h and use the code field? For e.g.

#define MEDIA_BUS_FMT_SDR_I2S_PADHI_BE   0x7001
#define MEDIA_BUS_FMT_SDR_I2S_PADHI_LE   0x7002

2) The framework does 

Re: [GIT PULL] Renesas ARM64 Based SoC DT Updates for v4.11

2017-01-10 Thread Geert Uytterhoeven
On Tue, Jan 10, 2017 at 8:54 AM, Simon Horman  wrote:
> On Mon, Jan 09, 2017 at 07:15:32PM -0800, Olof Johansson wrote:
>> On Fri, Jan 06, 2017 at 12:17:56PM +0100, Simon Horman wrote:
>> > 
>> > Chris Paterson (3):
>> >   arm64: dts: r8a7796: Add CAN external clock support
>> >   arm64: dts: r8a7796: Add CAN support
>> >   arm64: dts: r8a7796: Add CAN FD support
>> >
>> > Geert Uytterhoeven (2):
>> >   arm64: dts: r8a7796: Add all MSIOF nodes
>> >   arm64: renesas: r8a7796/salvator-x: Add board part number to DT 
>> > bindings
>>
>> Nit: When you look at the shortlog, these with different/random format
>> stand out. Feel free to fix them before sending next time.
>
> Sure, sorry for letting that slip through.

Note that the last one stands out because it's not a DTS update, but a DT
binding update, and thus follows a different convention ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH v2 0/6] usb: Replace the deprecated extcon API

2017-01-10 Thread Chanwoo Choi
Hi Felipe,

These patches have already acked-by tag from you.
Could you please apply them if there is additional comment?

On 2016년 12월 30일 13:08, Chanwoo Choi wrote:
> This patches just replace the deprecated extcon API without any change
> of extcon operation and use the resource-managed function for
> extcon_register_notifier().
> 
> The new extcon API instead of deprecated API.
> - extcon_set_cable_state_() -> extcon_set_state_sync();
> - extcon_get_cable_state_() -> extcon_get_state();
> 
> Changes from v1:
> - Rebase these patches based on v4.10-rc1.
> - Add acked-by tag from usb maintainer and reviewer.
> - Drop the phy/power-supply/chipidea patches.
> 
> Chanwoo Choi (6):
>   usb: dwc3: omap: Replace the extcon API
>   usb: phy: msm: Replace the extcon API
>   usb: phy: omap-otg: Replace the extcon API
>   usb: phy: qcom-8x16-usb: Replace the extcon API
>   usb: phy: tahvo: Replace the deprecated extcon API
>   usb: renesas_usbhs: Replace the deprecated extcon API
> 
>  drivers/usb/dwc3/dwc3-omap.c| 20 +++-
>  drivers/usb/phy/phy-msm-usb.c   | 33 +++--
>  drivers/usb/phy/phy-omap-otg.c  | 24 ++--
>  drivers/usb/phy/phy-qcom-8x16-usb.c | 13 -
>  drivers/usb/phy/phy-tahvo.c | 10 +-
>  drivers/usb/renesas_usbhs/common.c  |  2 +-
>  6 files changed, 34 insertions(+), 68 deletions(-)
> 


-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics


Re: [PATCH v2 0/2] phy: Replace the deprecated extcon API

2017-01-10 Thread Chanwoo Choi
Hi Kishon,

Could you review these patches or pick up them if there is no any comment?

On 2016년 12월 30일 13:11, Chanwoo Choi wrote:
> This patches just replace the deprecated extcon API without any change
> of extcon operation and use the resource-managed function for
> extcon_register_notifier().
> 
> The new extcon API instead of deprecated API.
> - extcon_set_cable_state_() -> extcon_set_state_sync();
> - extcon_get_cable_state_() -> extcon_get_state();
> 
> Changes from v1:
> - Rebase these patches based on v4.10-rc1.
> - Drop the usb/power-supply/chipidea patches.
> 
> Chanwoo Choi (2):
>   phy: rcar-gen3-usb2: Replace the deprecated extcon API
>   phy: sun4i-usb: Replace the deprecated extcon API
> 
>  drivers/phy/phy-rcar-gen3-usb2.c | 8 
>  drivers/phy/phy-sun4i-usb.c  | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 

-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics


Re: [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs

2017-01-10 Thread Simon Horman
On Thu, Jan 05, 2017 at 12:43:26PM +0100, Wolfram Sang wrote:
> Before enabling SDIO irqs, clear the status bit, so we discard old and
> stale interrupts. Needed to get two wireless cards working.
> 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/tmio_mmc_pio.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index d6033620a45c12..ebe3e12f0083dd 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -134,12 +134,21 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host 
> *mmc, int enable)
>   struct tmio_mmc_host *host = mmc_priv(mmc);
>  
>   if (enable && !host->sdio_irq_enabled) {
> + u16 sdio_status;
> +
>   /* Keep device active while SDIO irq is enabled */
>   pm_runtime_get_sync(mmc_dev(mmc));
> - host->sdio_irq_enabled = true;
>  
> + host->sdio_irq_enabled = true;
>   host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
>   ~TMIO_SDIO_STAT_IOIRQ;
> +
> + /* Clear obsolete interrupts before enabling */
> + sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS) & 
> ~TMIO_SDIO_MASK_ALL;
> + if (host->pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
> + sdio_status |= 6;

Perhaps a #define would be an improvement over "6".

> + sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
> +
>   sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
>   } else if (!enable && host->sdio_irq_enabled) {
>   host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> -- 
> 2.11.0
> 


Re: [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS

2017-01-10 Thread Simon Horman
On Thu, Jan 05, 2017 at 12:43:25PM +0100, Wolfram Sang wrote:
> QUIRK sounds like there is something wrong, but actually there are just
> some bits which need to be 1. Rename it to be more clear.
> 
> Signed-off-by: Wolfram Sang 
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c | 6 ++
>  drivers/mmc/host/tmio_mmc_pio.c   | 2 +-
>  include/linux/mfd/tmio.h  | 2 +-
>  3 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c 
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 59db14b4827c33..1600fefc2c3e2c 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -641,10 +641,8 @@ static int sh_mobile_sdhi_probe(struct platform_device 
> *pdev)
>*/
>   mmc_data->flags |= TMIO_MMC_HAVE_CMD12_CTRL;
>  
> - /*
> -  * All SDHI need SDIO_INFO1 reserved bit
> -  */
> - mmc_data->flags |= TMIO_MMC_SDIO_STATUS_QUIRK;
> + /* All SDHI have reserved bits */
> + mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
>  
>   ret = tmio_mmc_host_probe(host, mmc_data);
>   if (ret < 0)
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 60dc43b4574117..d6033620a45c12 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -723,7 +723,7 @@ static void __tmio_mmc_sdio_irq(struct tmio_mmc_host 
> *host)
>   ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdio_irq_mask;
>  
>   sdio_status = status & ~TMIO_SDIO_MASK_ALL;
> - if (pdata->flags & TMIO_MMC_SDIO_STATUS_QUIRK)
> + if (pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
>   sdio_status |= 6;
>  
>   sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index fba44abd05ba1a..5a05a34cc1a292 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -97,7 +97,7 @@
>  /*
>   * Some controllers needs to set 1 on SDIO status reserved bits
>   */

Is the following update to the above appropriate?
In particular, is it some or all?

* Controllers need to set 1 on SDIO status reserved bits

> -#define TMIO_MMC_SDIO_STATUS_QUIRK   (1 << 8)
> +#define TMIO_MMC_SDIO_STATUS_SETBITS (1 << 8)
>  
>  /*
>   * Some controllers have a 32-bit wide data port register
> -- 
> 2.11.0
> 


Re: [PATCH] arm64: dts: h3ulcb: follow sound CTU/MIX supports

2017-01-10 Thread Simon Horman
On Tue, Jan 10, 2017 at 07:41:28AM +, Kuninori Morimoto wrote:
> From: Kuninori Morimoto 
> 
> commit 5bcd74e8a30d9259 ("arm64: dts: r8a7795: add sound MIX support")
> commit 5be5ee41d011f26b ("arm64: dts: r8a7795: add sound CTU support")
> added MIX/CTU support, and it updated clocks on SoC level.
> Thus, h3ulcb should be updated
> 
> Signed-off-by: Kuninori Morimoto 

Thanks Morimoto-san,

applied.