Re: [PATCH] arm64: renesas: r8a7795: tidyup audma definition order

[Kuninori Morimoto]

Hi Simon

I want to know current status of this patch

> From: Kuninori Morimoto 
> 
> Current r8a7795.dtsi defines audma -> ipmmu -> dma order.
> Because of this order, dma can connect to ipmmu, but
> audma can't connect to it.
> This patch moves audma order as ipmmu -> dma -> audma.
> 
> Signed-off-by: Kuninori Morimoto 
> ---
>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 164 
> +++
>  1 file changed, 82 insertions(+), 82 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> index ede9f27..36c0b6e 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
> @@ -337,88 +337,6 @@
>   #power-domain-cells = <1>;
>   };
>  
> - audma0: dma-controller@ec70 {
> - compatible = "renesas,dmac-r8a7795",
> -  "renesas,rcar-dmac";
> - reg = <0 0xec70 0 0x1>;
> - interrupts =  -   GIC_SPI 320 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 321 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 322 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 323 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 324 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 327 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 328 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 330 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "error",
> - "ch0", "ch1", "ch2", "ch3",
> - "ch4", "ch5", "ch6", "ch7",
> - "ch8", "ch9", "ch10", "ch11",
> - "ch12", "ch13", "ch14", "ch15";
> - clocks = < CPG_MOD 502>;
> - clock-names = "fck";
> - power-domains = < R8A7795_PD_ALWAYS_ON>;
> - #dma-cells = <1>;
> - dma-channels = <16>;
> - iommus = <_mp1 0>, <_mp1 1>,
> -<_mp1 2>, <_mp1 3>,
> -<_mp1 4>, <_mp1 5>,
> -<_mp1 6>, <_mp1 7>,
> -<_mp1 8>, <_mp1 9>,
> -<_mp1 10>, <_mp1 11>,
> -<_mp1 12>, <_mp1 13>,
> -<_mp1 14>, <_mp1 15>;
> - };
> -
> - audma1: dma-controller@ec72 {
> - compatible = "renesas,dmac-r8a7795",
> -  "renesas,rcar-dmac";
> - reg = <0 0xec72 0 0x1>;
> - interrupts =  -   GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 338 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 339 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 344 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 346 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 347 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 348 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 349 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH
> -   GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-names = "error",
> - "ch0", "ch1", "ch2", "ch3",
> - "ch4", "ch5", "ch6", "ch7",
> - "ch8", "ch9", "ch10", "ch11",
> - "ch12", "ch13", "ch14", "ch15";
> -  

Re: [PATCH 1/2] net: phy: leds: Clear phy_num_led_triggers on failure to avoid crash

[Florian Fainelli]

On 01/24/2017 07:41 AM, Geert Uytterhoeven wrote:
> phy_attach_direct() ignores errors returned by
> phy_led_triggers_register(). I think that's OK, as LED triggers can be
> considered a non-critical feature.
> 
> However, this causes problems later:
>   - phy_led_trigger_change_speed() will access the array
> phy_device.phy_led_triggers, which has been freed in the error path
> of phy_led_triggers_register(), which may lead to a crash.
> 
>   - phy_led_triggers_unregister() will access the same array, leading to
> crashes during s2ram or poweroff, like:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address
>   
>   ...
>   [] (__list_del_entry_valid) from [] 
> (led_trigger_unregister+0x34/0xcc)
>   [] (led_trigger_unregister) from [] 
> (phy_led_triggers_unregister+0x28/0x34)
>   [] (phy_led_triggers_unregister) from [] 
> (phy_detach+0x30/0x74)
>   [] (phy_detach) from [] (sh_eth_close+0x64/0x9c)
>   [] (sh_eth_close) from [] 
> (dpm_run_callback+0x48/0xc8)
> 
> or:
> 
>   list_del corruption. prev->next should be dede6540, but was 2e323931
>   [ cut here ]
>   kernel BUG at lib/list_debug.c:52!
>   ...
>   [] (__list_del_entry_valid) from [] 
> (led_trigger_unregister+0x34/0xcc)
>   [] (led_trigger_unregister) from [] 
> (phy_led_triggers_unregister+0x28/0x34)
>   [] (phy_led_triggers_unregister) from [] 
> (phy_detach+0x30/0x74)
>   [] (phy_detach) from [] (sh_eth_close+0x6c/0xa4)
>   [] (sh_eth_close) from [] 
> (__dev_close_many+0xac/0xd0)
> 
> To fix this, clear phy_device.phy_num_led_triggers in the error path of
> phy_led_triggers_register() fails.
> 
> Note that the "No phy led trigger registered for speed" message will
> still be printed on link speed changes, which is a good cue that
> something went wrong with the LED triggers.
> 
> Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy 
> link state change")
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Florian Fainelli 
-- 
Florian

RE: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver

[Chris Brandt]

Hi Daniel,

On Tuesday, January 24, 2017, Daniel Lezcano wrote:
> > > > +early_platform_init("earlytimer", _timer);
> > > > +subsys_initcall(ostm_init); module_exit(ostm_exit);
> > > > +
> > > > +MODULE_AUTHOR("Chris Brandt");
> > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver");
> > > > +MODULE_LICENSE("GPL v2");
> > >
> > > Maybe you can try with builtin_platform ?
> >
> > Good idea. But, now I get a "Section mismatch" during link time so
> > I'll have to figure out why that is.
> 
> Mmh, I think it would be more consistent to convert this to:
> 
> CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init);
> 
> The only problem is to get the struct device associated to the of_node
> passed as parameter to ostm_init in order to use the devm_* API.
> 
> I think of_find_device_by_node should return the platform_device, then
> pdev->dev. If that works the other drivers will benefit from that to
> pdev->remove all
> the rollback code everywhere.


It was a good ideabut it will not work.

While CLOCKSOURCE_OF_DECLARE is good at putting the driver at the
front of the line for loading, it's too early in boot to detect
a platform_device.

of_find_device_by_node calls bus_find_device. But the first thing that
bus_find_device does is:

if (!bus || !bus->p)
return NULL;

But bus->p=0 so it returns immediately.


Of course changing the code over to:
devm_kzalloc -> devm_kzalloc
devm_ioremap_nocache -> of_io_request_and_map
platform_get_irq -> irq_of_parse_and_map
 dev_err -> pr_err


Then things work, but I'm back to managing the rollback code manually.


Any other ideas on how to get the corresponding platform_device for
a DT node?


Chris

Re: [PATCH 2/2] net: phy: leds: Fix truncated LED trigger names

[Andrew Lunn]

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5c9d2529685fe215..f6ab919528ab3627 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -25,7 +25,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  
>  #include 
>  
> @@ -339,6 +338,8 @@ struct phy_c45_device_ids {
>   u32 device_ids[8];
>  };
>  
> +#include 
> +
>  /* phy_device: An instance of a PHY
>   *
>   * drv: Pointer to the driver for this PHY instance
> diff --git a/include/linux/phy_led_triggers.h 
> b/include/linux/phy_led_triggers.h
> index a2daea0a37d2ae14..69dffb4fc5a294e9 100644
> --- a/include/linux/phy_led_triggers.h
> +++ b/include/linux/phy_led_triggers.h
> @@ -20,9 +20,8 @@
>  #include 
>  
>  #define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE10
> -#define PHY_MII_BUS_ID_SIZE  (20 - 3)
>  
> -#define PHY_LINK_LED_TRIGGER_NAME_SIZE (PHY_MII_BUS_ID_SIZE + \
> +#define PHY_LINK_LED_TRIGGER_NAME_SIZE (MII_BUS_ID_SIZE + \
>  FIELD_SIZEOF(struct mdio_device, addr)+\
>  PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE)

Hi Geert

Using the macro is great, but it does seem a bit ugly having the
include in the middle of the file.

As far as i can see, phy.h only uses a pointer to a struct
phy_led_trigger, not struct phy_led_trigger itself. Could you try
removing the header file all together and just have a forward
declaration of phy_led_trigger?

Thanks
Andrew

[PATCH V8 1/2] iio: adc: Add Renesas GyroADC bindings

[Marek Vasut]

Add DT bindings for the Renesas RCar GyroADC block. This block is
a simple 4/8-channel ADC which samples 12/15/24 bits of data every
cycle from all channels.

Signed-off-by: Marek Vasut 
Cc: Geert Uytterhoeven 
Cc: Simon Horman 
Cc: Jonathan Cameron 
Cc: linux-renesas-soc@vger.kernel.org
Cc: Wolfram Sang 
Cc: Rob Herring 
Cc: devicet...@vger.kernel.org
---
V8: - Sync the version with the 2/2 driver patch
- Drop status prop from example
- Add GyroADC block description
- Enumerate the compatible string values
- Make subnodes mandatory
---
 .../bindings/iio/adc/renesas,gyroadc.txt   | 99 ++
 1 file changed, 99 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt 
b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
new file mode 100644
index ..7f9b3bd120a9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
@@ -0,0 +1,99 @@
+* Renesas RCar GyroADC device driver
+
+The GyroADC block is a reduced SPI block with up to 8 chipselect lines,
+which supports the SPI protocol of a selected few SPI ADCs. The SPI ADCs
+are sampled by the GyroADC block in a round-robin fashion and the result
+presented in the GyroADC registers.
+
+Required properties:
+- compatible:  Should be "", "renesas,rcar-gyroadc".
+The  should be one of:
+   renesas,r8a7791-gyroadc - for the GyroADC block present
+ in r8a7791 SoC
+   renesas,r8a7792-gyroadc - for the GyroADC with interrupt
+ block present in r8a7792 SoC
+- reg: Address and length of the register set for the device
+- clocks:  References to all the clocks specified in the clock-names
+   property as specified in
+   Documentation/devicetree/bindings/clock/clock-bindings.txt.
+- clock-names: Shall contain "fck" and "if". The "fck" is the GyroADC block
+   clock, the "if" is the interface clock.
+- power-domains: Must contain a reference to the PM domain, if available.
+- #address-cells: Should be <1> (setting for the subnodes) for all ADCs
+   except for "fujitsu,mb88101a". Should be <0> (setting for
+   only subnode) for "fujitsu,mb88101a".
+- #size-cells: Should be <0> (setting for the subnodes)
+
+Sub-nodes:
+You must define subnode(s) which select the connected ADC type and reference
+voltage for the GyroADC channels.
+
+Required properties for subnodes:
+- compatible:  Should be either of:
+   "fujitsu,mb88101a"
+   - Fujitsu MB88101A compatible mode,
+ 12bit sampling, up to 4 channels can be sampled in
+ round-robin fashion. One Fujitsu chip supplies four
+ GyroADC channels with data as it contains four ADCs
+ on the chip and thus for 4-channel operation, single
+ MB88101A is required. The Cx chipselect lines of the
+ MB88101A connect directly to two CHS lines of the
+ GyroADC, no demuxer is required. The data out line
+ of each MB88101A connects to a shared input pin of
+ the GyroADC.
+   "ti,adcs7476" or "ti,adc121" or "adi,ad7476"
+   - TI ADCS7476 / TI ADC121 / ADI AD7476 compatible mode,
+ 15bit sampling, up to 8 channels can be sampled in
+ round-robin fashion. One TI/ADI chip supplies single
+ ADC channel with data, thus for 8-channel operation,
+ 8 chips are required. A 3:8 chipselect demuxer is
+ required to connect the nCS line of the TI/ADI chips
+ to the GyroADC, while MISO line of each TI/ADI ADC
+ connects to a shared input pin of the GyroADC.
+   "maxim,max1162" or "maxim,max11100"
+   - Maxim MAX1162 / Maxim MAX11100 compatible mode,
+ 16bit sampling, up to 8 channels can be sampled in
+ round-robin fashion. One Maxim chip supplies single
+ ADC channel with data, thus for 8-channel operation,
+ 8 chips are required. A 3:8 chipselect demuxer is
+ required to connect the nCS line of the MAX chips
+ to the GyroADC, while MISO line of each Maxim ADC
+ connects to a shared input pin of the GyroADC.
+- reg: Should be the number of the analog input. Should be present
+   for all 

[PATCH V8 2/2] iio: adc: Add Renesas GyroADC driver

[Marek Vasut]

Add IIO driver for the Renesas RCar GyroADC block. This block is a
simple 4/8-channel ADC which samples 12/15/24 bits of data every
cycle from all channels.

Signed-off-by: Marek Vasut 
Cc: Geert Uytterhoeven 
Cc: Simon Horman 
Cc: Jonathan Cameron 
Cc: linux-renesas-soc@vger.kernel.org
Cc: Wolfram Sang 
---
V2: - Spelling fixes
- Rename the driver source file to rcar-gyroadc
- Rework the channel sample width handling
- Use iio_device_claim_mode_direct()
- Rename "renesas,rcar-gyroadc" to "renesas,r8a7791-gyroadc" and
  rename "renesas,rcar-gyroadc-r8a7792" to "renesas,r8a7792-gyroadc"
  to match the new naming scheme (WARNING: scif uses the old one!)
- Switch to using regulators for channel voltage reference, add new
  properties renesas,gyroadc-vref-chN-supply for N in 0..7
- Handle vref regulators as optional to, make channels without
  vref regulator return EINVAL on read.
- Fix module license to GPL
- Drop interrupt.h include
- Rename clk to iclk
- Rename RCar to R-Car
- Rework the invalid mode handling
- Don't print error message on EPROBE_DEFER
- Drop fclk handling, use runtime PM for that instead
V3: - More R-Car spelling fixes
- Flip checks for V2H, since that's the only one that has
  interrupt registers
- Replace if-else statement with switch statement in init_mode
- Use unsigned types where applicable
- Rework timing calculation slightly to drop if-else block
- Use DIV_ROUND_CLOSEST
V4: - Add renesas,rcar-gyroadc fallback compatible string into the bindings
- Rework the ADC bindings to use per-channel subdevs
- Support more compatible ADC chips
V5: - Reword the DT bindings, improve the description of the modes
- Improve the description in Kconfig
- Implement special DT bindings for the MB88101A 4-channel ADC and
  add special handling for this chip into the driver.
- Add missing ADC stop on failure, wrap it into a function
- Fail hard in case the DT properties are incorrect or missing
- Fix reporting of scale, so that channel * scale value is in mV
- Use fractional scale instead of int plus nano
- Use pm_runtime to disable the sampling 2 seconds after last used
V6: - Replace the indio_dev->name with "rcar-gyroadc" instead of OF node name
V7: - Split the DT bindings out of this patch
V8: - No change
---
 MAINTAINERS|   6 +
 drivers/iio/adc/Kconfig|  13 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/rcar-gyroadc.c | 631 +
 4 files changed, 651 insertions(+)
 create mode 100644 drivers/iio/adc/rcar-gyroadc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0a2e5fe55249..7475db480519 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10445,6 +10445,12 @@ L: linux-renesas-soc@vger.kernel.org
 F: drivers/net/ethernet/renesas/
 F: include/linux/sh_eth.h
 
+RENESAS R-CAR GYROADC DRIVER
+M: Marek Vasut 
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/iio/adc/rcar_gyro_adc.c
+
 RENESAS USB2 PHY DRIVER
 M: Yoshihiro Shimoda 
 L: linux-renesas-soc@vger.kernel.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 9c8b558ba19e..2f7a8f3dbaf9 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -430,6 +430,19 @@ config QCOM_SPMI_VADC
  To compile this driver as a module, choose M here: the module will
  be called qcom-spmi-vadc.
 
+config RCAR_GYRO_ADC
+   tristate "Renesas R-Car GyroADC driver"
+   depends on ARCH_RCAR_GEN2 || (ARM && COMPILE_TEST)
+   help
+ Say yes here to build support for the GyroADC found in Renesas
+ R-Car Gen2 SoCs. This block is a simple SPI offload engine for
+ reading data out of attached compatible ADCs in a round-robin
+ fashion. Up to 4 or 8 ADC channels are supported by this block,
+ depending on which ADCs are attached.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rcar-gyroadc.
+
 config ROCKCHIP_SARADC
tristate "Rockchip SARADC driver"
depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be8d1fc..4bcd0edca4eb 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
 obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
+obj-$(CONFIG_RCAR_GYRO_ADC) += rcar-gyroadc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_STX104) += stx104.o
 obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
diff --git a/drivers/iio/adc/rcar-gyroadc.c 

[PATCH/RFC v3 net] ravb: unmap descriptors when freeing rings

[Simon Horman]

From: Kazuya Mizuguchi 

"swiotlb buffer is full" errors occur after repeated initialisation of a
device - f.e. suspend/resume or ip link set up/down. This is because memory
mapped using dma_map_single() in ravb_ring_format() and ravb_start_xmit()
is not released.  Resolve this problem by unmapping descriptors when
freeing rings.

Note, ravb_tx_free() is moved but not otherwise modified by this patch.

Signed-off-by: Kazuya Mizuguchi 
[simon: reworked]
Signed-off-by: Simon Horman 
--
v3 [Simon Horman]
* As suggested by Sergei Shtylyov
  - consistently use le32_to_cpu(desc->dptr)
  - Do not clear desc->ds_cc as it is not used
* Paramatise ravb_tx_free() to allow it to free non-transmitted buffers

v2 [Simon Horman]
* As suggested by Sergei Shtylyov
  - Use dma_mapping_error() and rx_desc->ds_cc when unmapping RX descriptors;
this is consistent with the way that they are mapped
  - Use ravb_tx_free() to clear TX descriptors
* Reduce scope of new local variable

v1 [Kazuya Mizuguchi]
---
 drivers/net/ethernet/renesas/ravb_main.c | 113 ++-
 1 file changed, 65 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
b/drivers/net/ethernet/renesas/ravb_main.c
index 89ac1e3f6175..57fe1411bb9d 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -179,6 +179,51 @@ static struct mdiobb_ops bb_ops = {
.get_mdio_data = ravb_get_mdio_data,
 };
 
+enum ravb_tx_free_mode {
+   ravb_tx_free_all,
+   ravb_tx_free_txed_only,
+};
+
+/* Free TX skb function for AVB-IP */
+static int ravb_tx_free(struct net_device *ndev, int q,
+   enum ravb_tx_free_mode free_mode)
+{
+   struct ravb_private *priv = netdev_priv(ndev);
+   struct net_device_stats *stats = >stats[q];
+   struct ravb_tx_desc *desc;
+   int free_num = 0;
+   int entry;
+   u32 size;
+
+   for (; priv->cur_tx[q] - priv->dirty_tx[q] > 0; priv->dirty_tx[q]++) {
+   entry = priv->dirty_tx[q] % (priv->num_tx_ring[q] *
+NUM_TX_DESC);
+   desc = >tx_ring[q][entry];
+   if (free_mode == ravb_tx_free_txed_only &&
+   desc->die_dt != DT_FEMPTY)
+   break;
+   /* Descriptor type must be checked before all other reads */
+   dma_rmb();
+   size = le16_to_cpu(desc->ds_tagl) & TX_DS;
+   /* Free the original skb. */
+   if (priv->tx_skb[q][entry / NUM_TX_DESC]) {
+   dma_unmap_single(ndev->dev.parent, 
le32_to_cpu(desc->dptr),
+size, DMA_TO_DEVICE);
+   /* Last packet descriptor? */
+   if (entry % NUM_TX_DESC == NUM_TX_DESC - 1) {
+   entry /= NUM_TX_DESC;
+   dev_kfree_skb_any(priv->tx_skb[q][entry]);
+   priv->tx_skb[q][entry] = NULL;
+   stats->tx_packets++;
+   }
+   free_num++;
+   }
+   stats->tx_bytes += size;
+   desc->die_dt = DT_EEMPTY;
+   }
+   return free_num;
+}
+
 /* Free skb's and DMA buffers for Ethernet AVB */
 static void ravb_ring_free(struct net_device *ndev, int q)
 {
@@ -194,19 +239,21 @@ static void ravb_ring_free(struct net_device *ndev, int q)
kfree(priv->rx_skb[q]);
priv->rx_skb[q] = NULL;
 
-   /* Free TX skb ringbuffer */
-   if (priv->tx_skb[q]) {
-   for (i = 0; i < priv->num_tx_ring[q]; i++)
-   dev_kfree_skb(priv->tx_skb[q][i]);
-   }
-   kfree(priv->tx_skb[q]);
-   priv->tx_skb[q] = NULL;
-
/* Free aligned TX buffers */
kfree(priv->tx_align[q]);
priv->tx_align[q] = NULL;
 
if (priv->rx_ring[q]) {
+   for (i = 0; i < priv->num_rx_ring[q]; i++) {
+   struct ravb_ex_rx_desc *desc = >rx_ring[q][i];
+
+   if (!dma_mapping_error(ndev->dev.parent,
+  le32_to_cpu(desc->dptr)))
+   dma_unmap_single(ndev->dev.parent,
+le32_to_cpu(desc->dptr),
+PKT_BUF_SZ,
+DMA_FROM_DEVICE);
+   }
ring_size = sizeof(struct ravb_ex_rx_desc) *
(priv->num_rx_ring[q] + 1);
dma_free_coherent(ndev->dev.parent, ring_size, priv->rx_ring[q],
@@ -215,12 +262,19 @@ static void ravb_ring_free(struct net_device *ndev, int q)
}
 
if (priv->tx_ring[q]) {
+   ravb_tx_free(ndev, q, 

Re: [PATCH 1/2] iio: adc: Add Renesas GyroADC bindings

[Geert Uytterhoeven]

Hi Marek,

On Tue, Jan 24, 2017 at 5:39 PM, Marek Vasut  wrote:
> On 01/24/2017 08:30 AM, Geert Uytterhoeven wrote:
>> On Tue, Jan 24, 2017 at 12:15 AM, Marek Vasut  wrote:
>>> What about this ?
>>>
>>> - compatible:   Should be "renesas,-gyroadc", "renesas,rcar-gyroadc".
>>> The  can be either of:
>>> r8a7791 - for the GyroADC block present in r8a7791 SoC
>>> r8a7792 - for the GyroADC with interrupt block present
>>>   in r8a7792 SoC
>>
>> No, we need the exact string in the document, cfr. all other Renesas 
>> bindings.
>>
>> "renesas,r8a7791-gyroadc"
>> "renesas,r8a7792-gyroadc"
>
> This ?
>
> - compatible:   Should be "", "renesas,rcar-gyroadc".
> The  should be one of:
> renesas,r8a7791-gyroadc - for the GyroADC block present
>   in r8a7791 SoC
> renesas,r8a7792-gyroadc - for the GyroADC with interrupt
>   block present in r8a7792 SoC

Yes. I had expected that double quotes were mandatory, but apparently
checkpatch doesn't require that.

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] iio: adc: Add Renesas GyroADC bindings

[Marek Vasut]

On 01/24/2017 08:30 AM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Tue, Jan 24, 2017 at 12:15 AM, Marek Vasut  wrote:
>> On 01/23/2017 09:41 PM, Geert Uytterhoeven wrote:
>>> On Mon, Jan 23, 2017 at 8:56 PM, Marek Vasut  wrote:
 On 01/23/2017 06:08 PM, Rob Herring wrote:
> On Sat, Jan 21, 2017 at 03:42:11PM +0100, Marek Vasut wrote:
>> new file mode 100644
>> index ..081947367135
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>>>
>> +Required properties:
>> +- compatible:   Should be "renesas,-gyroadc", 
>> "renesas,rcar-gyroadc".
>
> Need to enumerate .

 It's enumerated below , the only special-case we handle at this point is
 the r8a7792 , which has extra interrupt block which we don't use anyway,
 but the driver disables those interrupts.
>>>
>>> It is not enumerated below. You just give an example for r8a7791.
>>>
>>> Please list all supported/tested compatible values, so checkpatch
>>> can validate compatible values in DTS patches adding GyroADC device
>>> nodes.
>>
>> What about this ?
>>
>> - compatible:   Should be "renesas,-gyroadc", "renesas,rcar-gyroadc".
>> The  can be either of:
>> r8a7791 - for the GyroADC block present in r8a7791 SoC
>> r8a7792 - for the GyroADC with interrupt block present
>>   in r8a7792 SoC
> 
> No, we need the exact string in the document, cfr. all other Renesas bindings.
> 
> "renesas,r8a7791-gyroadc"
> "renesas,r8a7792-gyroadc"

This ?

- compatible:   Should be "", "renesas,rcar-gyroadc".
The  should be one of:
renesas,r8a7791-gyroadc - for the GyroADC block present
  in r8a7791 SoC
renesas,r8a7792-gyroadc - for the GyroADC with interrupt
  block present in r8a7792 SoC

-- 
Best regards,
Marek Vasut

RE: [PATCH] ARM: dts: r7s72100: add power-domains to mmcif

[Chris Brandt]

On Tuesday, January 24, 2017, Simon Horman wrote:
> On Mon, Jan 23, 2017 at 04:12:16PM +0100, Geert Uytterhoeven wrote:
> > On Mon, Jan 23, 2017 at 3:13 PM, Chris Brandt 
> wrote:
> > > Signed-off-by: Chris Brandt 
> >
> > Reported-by: Geert Uytterhoeven 
> > Acked-by: Geert Uytterhoeven 
> 
> Should this be queued up as a fix for v4.10 with the following tag?
> 
> Fixes: 887862227ba3 ("ARM: dts: r7s72100: add mmcif to device tree")

I assume it can. I'm not sure how it effect the driver one way good or bad
(that's a question for Geert I think).
SDHI for r7s72100 needs it too, I just didn't send that yet because my other
fix will cause a merge conflict because it is modifying the same line. But, 
that patch is taking longer than I expected.

My guess is that power-domains is for runtime power management...which seems
to be a sore subject for r7s72100 at this point.


Chris

About DU DRM

[Stuvart S]

Hello Laurent and all,

   I am tracking complete files of DU in my source tree linux 4.9
one by one .I know it is not an easy task to study completely  but now
it is really interesting for me..

first of all I am saying my ultimate goal here,I want to get track the
 whole route of
image/video data until it reaches to final output (Display) ..till now
I understand the flow as below

FRAME BUFFER > CRTC --> ENCODER -->CONNECTOR->DISPLAY

By familiarizing DRM I got following knowledge,

mainly DRM has its Core(Generic ) and Driver(Specific)..for both of
these there are different ioctls. and in the first case most of the
ioctls are wrapped through libdrm functions for easiness ..

hopping suggestions if I am in wrong path..

On the other side, I think driver dependent APIs will be there right?

How various graphics drivers will communicate to DRM?
In my limited knowledge, DRM is purely software centric right? so the
communication will be through some APIs I think..

expecting a helping hand

-- 
--- ComeLet's enjoy the world of Open Source  ---


Best regards,
Stuvart

RE: [RFC] Runtime PM on the RZ/A series is never going to work right

[Chris Brandt]

Hi Geert,

On Tuesday, January 24, 2017, Geert Uytterhoeven wrote:
> > From what I can tell, that makes the register space readable...but the
> > IP block is not fully functional unless you delay a little.
> 
> If you know the minimum delay needed, and it's not too long, it can be
> added to the enable path.

I can play around and see. I know udealy(100) works OK, but then I have
to have a delay that's as long as the slowest peripheral.
If it was just to turn a clock on once, or once in a while, that's OK. But
it seems like runtime pm wants to turn the clocks on/off as much as
possible.

> > But, I think I'd like to disable runtime pm for RZ/A1 in the drivers
> > for now Because 'functional' is better than 'lower-power-but-broken'
> 
> I prefer not doing that in the individual drivers, as they're shared with
> other SoCs.

What I meant was looking at the compatible string and only doing
it for RZ/A1.

For example, in rspi_probe():

if (of_device_is_compatible(np, " renesas,rspi-r7s72100"))
master->auto_runtime_pm = false;
else
master->auto_runtime_pm = true;


I would do the same kind of thing for riic.



> I think you can handle that in drivers/clk/renesas/clk-mstp.c:
>   - in cpg_mstp_attach_dev(), add a call to pm_clk_resume(dev) after the
> call to pm_clk_add_clk(),
>   - in cpg_mstp_detach_dev(), add a call to pm_clk_suspend(dev) before the
> call to pm_clk_destroy().
> Yes, that means the module clocks are enabled all the time.
> Of course when running on RZ/A1H ;-)

That might be OK.

But won't the individual drivers still want to keep turning clocks on and off 
manually?
(unless I'm not understanding that the underlying clock routines will basically 
just
ignore everything). But even if that' the case...that just wasted CPU cycles 
(remember,
I'm only working with a 400MHz single core here running XIP_KERNEL)



I think I should at least put the dummy read in cpg_mstp_clock_endisable() 
first,
then maybe I can see what drivers work. Something like this:


spin_lock_irqsave(>lock, flags);

value = cpg_mstp_read(group, group->smstpcr);
if (enable)
value &= ~bitmask;
else
value |= bitmask;
cpg_mstp_write(group, value, group->smstpcr);

+   if (!group->mstpsr) {
+   /* dummy read to ensure write has completed */
+   clk_readl(group->smstpcr);
+   barrier_data(group->smstpcr);
+   }

spin_unlock_irqrestore(>lock, flags);



Chris

Re: [PATCH] drivers: firmware: psci: Use __pa_symbol for cpu_resume

[Will Deacon]

On Tue, Jan 24, 2017 at 03:33:51PM +, Mark Rutland wrote:
> On Tue, Jan 24, 2017 at 04:30:19PM +0100, Geert Uytterhoeven wrote:
> > If CONFIG_DEBUG_VIRTUAL=y, during s2ram:
> > 
> > virt_to_phys used for non-linear address: ff80085db280 
> > (cpu_resume+0x0/0x20)
> > [ cut here ]
> > WARNING: CPU: 0 PID: 1628 at arch/arm64/mm/physaddr.c:14 
> > __virt_to_phys+0x28/0x60
> > ...
> > [] __virt_to_phys+0x28/0x60
> > [] psci_system_suspend+0x20/0x44
> > [] cpu_suspend+0x3c/0x68
> > [] psci_system_suspend_enter+0x18/0x20
> > [] suspend_devices_and_enter+0x3f8/0x7e8
> > [] pm_suspend+0x544/0x5f4
> > 
> > Fixes: 1a08e3d9e0ac4577 ("drivers: firmware: psci: Use __pa_symbol for 
> > kernel symbol")
> > Signed-off-by: Geert Uytterhoeven 
> 
> Argh, I should have spotted this. :(
> 
> Acked-by: Mark Rutland 
> 
> This should go via the arm64 tree -- Will, could you pick this up? 

Yup, I'll queue this along with the other __pa_symbol fix.

Cheers,

Will

Re: [PATCH 3/3] sh_eth: stop using bare numbers for EESIPR values

[Sergei Shtylyov]

On 01/23/2017 11:00 AM, Geert Uytterhoeven wrote:


Now  that we  have almost all EESIPR bits declared (and those that  are
still not are most probably reserved anyway) we can at last replace the
bare  numbers used for 'sh_eth_cpu_data::eesipr_value' initializers with
the bit names ORed together...

Signed-off-by: Sergei Shtylyov 

---
 drivers/net/ethernet/renesas/sh_eth.c |   89 +-
 1 file changed, 78 insertions(+), 11 deletions(-)

Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c



@@ -800,7 +843,12 @@ static struct sh_eth_cpu_data sh7734_dat

.ecsr_value = ECSR_ICD | ECSR_MPD,
.ecsipr_value   = ECSIPR_LCHNGIP | ECSIPR_ICDIP | ECSIPR_MPDIP,
-   .eesipr_value   = EESIPR_RFCOFIP | EESIPR_ECIIP | 0x003f07ff,
+   .eesipr_value   = EESIPR_RFCOFIP | EESIPR_ECIIP |
+ EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
+ EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |


Missing:

EESIPR_DLCIP | EESIPR_CDIP | EESIPR_TROIP |


   Hm, I thought I'd double-checked all the initializers... TY!


+ EESIPR_RMAFIP | EESIPR_CEEFIP | EESIPR_CELFIP |
+ EESIPR_RRFIP | EESIPR_RTLFIP | EESIPR_RTSFIP |
+ EESIPR_PREIP | EESIPR_CERFIP,

.tx_check   = EESR_TC1 | EESR_FTC,
.eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
@@ -830,7 +878,12 @@ static struct sh_eth_cpu_data sh7763_dat

.ecsr_value = ECSR_ICD | ECSR_MPD,
.ecsipr_value   = ECSIPR_LCHNGIP | ECSIPR_ICDIP | ECSIPR_MPDIP,
-   .eesipr_value   = EESIPR_RFCOFIP | EESIPR_ECIIP | 0x003f07ff,
+   .eesipr_value   = EESIPR_RFCOFIP | EESIPR_ECIIP |
+ EESIPR_FTCIP | EESIPR_TDEIP | EESIPR_TFUFIP |
+ EESIPR_FRIP | EESIPR_RDEIP | EESIPR_RFOFIP |


Likewise


   :-<


+ EESIPR_RMAFIP | EESIPR_CEEFIP | EESIPR_CELFIP |
+ EESIPR_RRFIP | EESIPR_RTLFIP | EESIPR_RTSFIP |
+ EESIPR_PREIP | EESIPR_CERFIP,


Reviewed-by: Geert Uytterhoeven 


   Will add, thanks again...


Gr{oetje,eeting}s,

Geert


MBR, Sergei

Re: [PATCH 1/3] sh_eth: rename EESIPR bits

[Sergei Shtylyov]

Hello!

On 01/23/2017 10:39 AM, Geert Uytterhoeven wrote:


Since the  commit  b0ca2a21f769 ("sh_eth: Add support of SH7763 to sh_eth")
the *enum* declaring the EESIPR bits (interrupt mask) went out of sync with
the *enum* declaring the EESR bits (interrupt status) WRT  bit naming  and
formatting. I'd like to restore the consistency by using EESIPR as the bit
name prefix, renaming the *enum* to EESIPR_BIT, and (finally) renaming the
bits according to the available  Renesas SH77{34|63} manuals...


Which versions of the SH77{34|63} manuals did you use?
Several registers are called slightly different in mine, and also in my
r8a7740 manual.


   SH7734 User's Manual: Hardware, Rev.1.00 Jun 2012.
   SH7763 User’s Manual: Hardware, Rev.3.00 Mar 2012


--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ net-next/drivers/net/ethernet/renesas/sh_eth.h
@@ -268,19 +268,29 @@ enum EESR_BIT {
 EESR_TFE | EESR_TDE)

 /* EESIPR */
-enum DMAC_IM_BIT {
-   DMAC_M_TWB = 0x4000, DMAC_M_TABT = 0x0400,
-   DMAC_M_RABT = 0x0200,
-   DMAC_M_RFRMER = 0x0100, DMAC_M_ADF = 0x0080,
-   DMAC_M_ECI = 0x0040, DMAC_M_FTC = 0x0020,
-   DMAC_M_TDE = 0x0010, DMAC_M_TFE = 0x0008,
-   DMAC_M_FRC = 0x0004, DMAC_M_RDE = 0x0002,
-   DMAC_M_RFE = 0x0001, DMAC_M_TINT4 = 0x0800,
-   DMAC_M_TINT3 = 0x0400, DMAC_M_TINT2 = 0x0200,
-   DMAC_M_TINT1 = 0x0100, DMAC_M_RINT8 = 0x0080,
-   DMAC_M_RINT5 = 0x0010, DMAC_M_RINT4 = 0x0008,
-   DMAC_M_RINT3 = 0x0004, DMAC_M_RINT2 = 0x0002,
-   DMAC_M_RINT1 = 0x0001,
+enum EESIPR_BIT {
+   EESIPR_TWBIP= 0x4000,


TWBIP is actually two bits in my manual: TWB1IP and TWB0IP


   Yes. I'm adding TWB1 in the next patch.


+   EESIPR_ADEIP= 0x0080,


Nonexistent bit in my manual.


   In mine as well. I used EESR *enum* as a source of info in this case.
Note that this bit is documented for R-Car gen2 (but called differently).


+   EESIPR_CNDIP= 0x0800,


Nonexistent bit in my manual.


   I used the EESR *enum* again.


Gr{oetje,eeting}s,

Geert


MBR, Sergei

Re: [PATCH 2/3] sh_eth: add missing EESIPR bits

[Sergei Shtylyov]

On 01/23/2017 10:41 AM, Geert Uytterhoeven wrote:


Renesas SH77{34|63} manuals  describe more EESIPR bits than the current
driver. Declare the new bits with the end goal of using the bit names
instead of the bare numbers  for  the 'sh_eth_cpu_data::eesipr_value'
initializers...

Signed-off-by: Sergei Shtylyov 


Reviewed-by: Geert Uytterhoeven 


---
 drivers/net/ethernet/renesas/sh_eth.h |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

Index: net-next/drivers/net/ethernet/renesas/sh_eth.h
===
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.h
+++ net-next/drivers/net/ethernet/renesas/sh_eth.h
@@ -269,13 +269,17 @@ enum EESR_BIT {

 /* EESIPR */
 enum EESIPR_BIT {
-   EESIPR_TWBIP= 0x4000,
+   EESIPR_TWB1IP   = 0x8000,
+   EESIPR_TWBIP= 0x4000,   /* same as TWB0IP */


Ah, your adding it here ;-)


   Do you want me to add such comment in the 1st patch instead?


Gr{oetje,eeting}s,

Geert


MRB, Sergei

[PATCH 0/2] net: phy: leds: Fix truncated LED trigger names and crashes

[Geert Uytterhoeven]

Hi David,

I started seeing crashes during s2ram and poweroff on all my ARM boards,
like:

Unable to handle kernel NULL pointer dereference at virtual address 
...
[] (__list_del_entry_valid) from [] 
(led_trigger_unregister+0x34/0xcc)
[] (led_trigger_unregister) from [] 
(phy_led_triggers_unregister+0x28/0x34)
[] (phy_led_triggers_unregister) from [] 
(phy_detach+0x30/0x74)
[] (phy_detach) from [] (sh_eth_close+0x64/0x9c)
[] (sh_eth_close) from [] (dpm_run_callback+0x48/0xc8)

or:

list_del corruption. prev->next should be dede6540, but was 2e323931
[ cut here ]
kernel BUG at lib/list_debug.c:52!
...
[] (__list_del_entry_valid) from [] 
(led_trigger_unregister+0x34/0xcc)
[] (led_trigger_unregister) from [] 
(phy_led_triggers_unregister+0x28/0x34)
[] (phy_led_triggers_unregister) from [] 
(phy_detach+0x30/0x74)
[] (phy_detach) from [] (sh_eth_close+0x6c/0xa4)
[] (sh_eth_close) from [] (__dev_close_many+0xac/0xd0)

As the only clue was a kernel message like

sh-eth ee70.ethernet eth0: No phy led trigger registered for speed(100)

I had to bisected this, leading to commit 4567d686f5c6d955 ("phy:
increase size of MII_BUS_ID_SIZE and bus_id").  Reverting that commit
fixed the issue.

More investigation revealed the crashes are due to the combination of
two things:
  - Truncated LED trigger names, leading to duplicate names, and
registration failures,
  - Bad error handling in case of registration failures.

Both are fixed by this patch series.

Thanks!

Geert Uytterhoeven (2):
  net: phy: leds: Clear phy_num_led_triggers on failure to avoid crash
  net: phy: leds: Fix truncated LED trigger names

 drivers/net/phy/phy_led_triggers.c | 8 ++--
 include/linux/phy.h| 3 ++-
 include/linux/phy_led_triggers.h   | 3 +--
 3 files changed, 9 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[PATCH 2/2] net: phy: leds: Fix truncated LED trigger names

[Geert Uytterhoeven]

Commit 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and
bus_id") increased the size of MII bus IDs, but forgot to update the
private definition in .
This may cause:
  1. Truncation of LED trigger names,
  2. Duplicate LED trigger names,
  3. Failures registering LED triggers,
  4. Crashes due to bad error handling in the LED trigger failure path.

To fix this, and prevent the definitions going out of sync again in the
future, let the PHY LED trigger code use the existing MII_BUS_ID_SIZE
definition. Note that this requires moving the #include down, after the
definition of MII_BUS_ID_SIZE.

Example:
  - Before I had triggers "ee70.etherne:01:100Mbps" and
"ee70.etherne:01:10Mbps",
  - After the increase of MII_BUS_ID_SIZE, both became
"ee70.ethernet-:01:" => FAIL,
  - Now, the triggers are "ee70.ethernet-:01:100Mbps" and
"ee70.ethernet-:01:10Mbps", which are unique again.

Fixes: 4567d686f5c6d955 ("phy: increase size of MII_BUS_ID_SIZE and bus_id")
Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy 
link state change")
Signed-off-by: Geert Uytterhoeven 
---
 include/linux/phy.h  | 3 ++-
 include/linux/phy_led_triggers.h | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 5c9d2529685fe215..f6ab919528ab3627 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #include 
 
@@ -339,6 +338,8 @@ struct phy_c45_device_ids {
u32 device_ids[8];
 };
 
+#include 
+
 /* phy_device: An instance of a PHY
  *
  * drv: Pointer to the driver for this PHY instance
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h
index a2daea0a37d2ae14..69dffb4fc5a294e9 100644
--- a/include/linux/phy_led_triggers.h
+++ b/include/linux/phy_led_triggers.h
@@ -20,9 +20,8 @@
 #include 
 
 #define PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE  10
-#define PHY_MII_BUS_ID_SIZE(20 - 3)
 
-#define PHY_LINK_LED_TRIGGER_NAME_SIZE (PHY_MII_BUS_ID_SIZE + \
+#define PHY_LINK_LED_TRIGGER_NAME_SIZE (MII_BUS_ID_SIZE + \
   FIELD_SIZEOF(struct mdio_device, addr)+\
   PHY_LED_TRIGGER_SPEED_SUFFIX_SIZE)
 
-- 
1.9.1

[PATCH 1/2] net: phy: leds: Clear phy_num_led_triggers on failure to avoid crash

[Geert Uytterhoeven]

phy_attach_direct() ignores errors returned by
phy_led_triggers_register(). I think that's OK, as LED triggers can be
considered a non-critical feature.

However, this causes problems later:
  - phy_led_trigger_change_speed() will access the array
phy_device.phy_led_triggers, which has been freed in the error path
of phy_led_triggers_register(), which may lead to a crash.

  - phy_led_triggers_unregister() will access the same array, leading to
crashes during s2ram or poweroff, like:

Unable to handle kernel NULL pointer dereference at virtual address

...
[] (__list_del_entry_valid) from [] 
(led_trigger_unregister+0x34/0xcc)
[] (led_trigger_unregister) from [] 
(phy_led_triggers_unregister+0x28/0x34)
[] (phy_led_triggers_unregister) from [] 
(phy_detach+0x30/0x74)
[] (phy_detach) from [] (sh_eth_close+0x64/0x9c)
[] (sh_eth_close) from [] 
(dpm_run_callback+0x48/0xc8)

or:

list_del corruption. prev->next should be dede6540, but was 2e323931
[ cut here ]
kernel BUG at lib/list_debug.c:52!
...
[] (__list_del_entry_valid) from [] 
(led_trigger_unregister+0x34/0xcc)
[] (led_trigger_unregister) from [] 
(phy_led_triggers_unregister+0x28/0x34)
[] (phy_led_triggers_unregister) from [] 
(phy_detach+0x30/0x74)
[] (phy_detach) from [] (sh_eth_close+0x6c/0xa4)
[] (sh_eth_close) from [] 
(__dev_close_many+0xac/0xd0)

To fix this, clear phy_device.phy_num_led_triggers in the error path of
phy_led_triggers_register() fails.

Note that the "No phy led trigger registered for speed" message will
still be printed on link speed changes, which is a good cue that
something went wrong with the LED triggers.

Fixes: 2e0bc452f4721520 ("net: phy: leds: add support for led triggers on phy 
link state change")
Signed-off-by: Geert Uytterhoeven 
---
Alternatively, phy_attach_direct() could consider
phy_led_triggers_register() failures as fatal, so
phy_led_trigger_change_speed() and phy_led_triggers_unregister() are
never called afterwards.

Exposed by commit 4567d686f5c6d955 ("phy: increase size of
MII_BUS_ID_SIZE and bus_id"), which caused duplicate trigger names.
---
 drivers/net/phy/phy_led_triggers.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_led_triggers.c 
b/drivers/net/phy/phy_led_triggers.c
index fa62bdf2f52694de..3f619e7371e97d8a 100644
--- a/drivers/net/phy/phy_led_triggers.c
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -102,8 +102,10 @@ int phy_led_triggers_register(struct phy_device *phy)
sizeof(struct phy_led_trigger) *
   phy->phy_num_led_triggers,
GFP_KERNEL);
-   if (!phy->phy_led_triggers)
-   return -ENOMEM;
+   if (!phy->phy_led_triggers) {
+   err = -ENOMEM;
+   goto out_clear;
+   }
 
for (i = 0; i < phy->phy_num_led_triggers; i++) {
err = phy_led_trigger_register(phy, >phy_led_triggers[i],
@@ -120,6 +122,8 @@ int phy_led_triggers_register(struct phy_device *phy)
while (i--)
phy_led_trigger_unregister(>phy_led_triggers[i]);
devm_kfree(>mdio.dev, phy->phy_led_triggers);
+out_clear:
+   phy->phy_num_led_triggers = 0;
return err;
 }
 EXPORT_SYMBOL_GPL(phy_led_triggers_register);
-- 
1.9.1

Re: [PATCH] drivers: firmware: psci: Use __pa_symbol for cpu_resume

[Mark Rutland]

On Tue, Jan 24, 2017 at 04:30:19PM +0100, Geert Uytterhoeven wrote:
> If CONFIG_DEBUG_VIRTUAL=y, during s2ram:
> 
> virt_to_phys used for non-linear address: ff80085db280 
> (cpu_resume+0x0/0x20)
> [ cut here ]
> WARNING: CPU: 0 PID: 1628 at arch/arm64/mm/physaddr.c:14 
> __virt_to_phys+0x28/0x60
> ...
> [] __virt_to_phys+0x28/0x60
> [] psci_system_suspend+0x20/0x44
> [] cpu_suspend+0x3c/0x68
> [] psci_system_suspend_enter+0x18/0x20
> [] suspend_devices_and_enter+0x3f8/0x7e8
> [] pm_suspend+0x544/0x5f4
> 
> Fixes: 1a08e3d9e0ac4577 ("drivers: firmware: psci: Use __pa_symbol for kernel 
> symbol")
> Signed-off-by: Geert Uytterhoeven 

Argh, I should have spotted this. :(

Acked-by: Mark Rutland 

This should go via the arm64 tree -- Will, could you pick this up? 

Thanks,
Mark.

> ---
>  drivers/firmware/psci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 66a8793f3b3793ff..493a56a4cfc4a836 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -419,7 +419,7 @@ int psci_cpu_suspend_enter(unsigned long index)
>  static int psci_system_suspend(unsigned long unused)
>  {
>   return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
> -   virt_to_phys(cpu_resume), 0, 0);
> +   __pa_symbol(cpu_resume), 0, 0);
>  }
>  
>  static int psci_system_suspend_enter(suspend_state_t state)
> -- 
> 1.9.1
> 

[PATCH] drivers: firmware: psci: Use __pa_symbol for cpu_resume

[Geert Uytterhoeven]

If CONFIG_DEBUG_VIRTUAL=y, during s2ram:

virt_to_phys used for non-linear address: ff80085db280 
(cpu_resume+0x0/0x20)
[ cut here ]
WARNING: CPU: 0 PID: 1628 at arch/arm64/mm/physaddr.c:14 
__virt_to_phys+0x28/0x60
...
[] __virt_to_phys+0x28/0x60
[] psci_system_suspend+0x20/0x44
[] cpu_suspend+0x3c/0x68
[] psci_system_suspend_enter+0x18/0x20
[] suspend_devices_and_enter+0x3f8/0x7e8
[] pm_suspend+0x544/0x5f4

Fixes: 1a08e3d9e0ac4577 ("drivers: firmware: psci: Use __pa_symbol for kernel 
symbol")
Signed-off-by: Geert Uytterhoeven 
---
 drivers/firmware/psci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index 66a8793f3b3793ff..493a56a4cfc4a836 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -419,7 +419,7 @@ int psci_cpu_suspend_enter(unsigned long index)
 static int psci_system_suspend(unsigned long unused)
 {
return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
- virt_to_phys(cpu_resume), 0, 0);
+ __pa_symbol(cpu_resume), 0, 0);
 }
 
 static int psci_system_suspend_enter(suspend_state_t state)
-- 
1.9.1

RE: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver

[Chris Brandt]

Hi Daniel,

On Tuesday, January 24, 2017, Daniel Lezcano wrote:
> > > > +early_platform_init("earlytimer", _timer);
> > > > +subsys_initcall(ostm_init); module_exit(ostm_exit);
> > > > +
> > > > +MODULE_AUTHOR("Chris Brandt");
> > > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver");
> > > > +MODULE_LICENSE("GPL v2");
> > >
> > > Maybe you can try with builtin_platform ?
> >
> > Good idea. But, now I get a "Section mismatch" during link time so
> > I'll have to figure out why that is.
> 
> Mmh, I think it would be more consistent to convert this to:
> 
> CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init);
> 
> The only problem is to get the struct device associated to the of_node
> passed as parameter to ostm_init in order to use the devm_* API.
> 
> I think of_find_device_by_node should return the platform_device, then
> pdev->dev. If that works the other drivers will benefit from that to
> pdev->remove all
> the rollback code everywhere.


So I realized that in order to use builtin_platform, I can't have any of the
functions in __init because the build system has no idea that I never plan
on removing or probing again after boot. But, even if I take out all the
__init from my code, I'm still calling clocksource_mmio_init which is __init
so I can never escape the "Section mismatch".

This leaves me with 2 options:

1) Pull the clocksource_mmio driver code into my driver and leave everything
   out of __init
2) Rewrite as a DT driver using CLOCKSOURCE_OF_DECLARE which puts most of
   the code in __init

Of course the better (lower system memory) answer is #2.

I'll try your trick about of_find_device_by_node because while there are
equivalent OF functions for mapping resources, I do like the auto-rollback
feature of demv

Thank you,
Chris

Re: [RFC] Runtime PM on the RZ/A series is never going to work right

[Geert Uytterhoeven]

Hi Chris,

On Tue, Jan 24, 2017 at 3:20 PM, Chris Brandt  wrote:
> On Tuesday, January 24, 2017, Geert Uytterhoeven wrote:
>> > Therefore, before I start firing off patches to remove runtime PM for
>> RZ/A, does anyone have an opinion one way of the other
>>
>> Please have a look at Section 55.3.5 ("Module Standby Function"), which I
>> had never read myself, apparently...
>>
>> Seem like there is some kind of status register, the STBCR register
>> itself:
>>
>> Canceling Module Standby Function:
>> 1. Clear the MSTP bit to 0.
>> 2. After that, dummy-read the same register.
>>
>> Does it help if you add the dummy read when enabling the clock?
>
> Already tried that. And put a barrier() call just to make sure
> everything was done.
> It seems that might be enough for the RSPI, but the RIIC still has issues.

:-(

> From what I can tell, that makes the register space readable...but the IP 
> block
> is not fully functional unless you delay a little.

If you know the minimum delay needed, and it's not too long, it can be
added to the enable path.

>> However, that section reveals another complicating factor for some
>> modules:
>> if the module has a bit in a STBREQn/STBACKn register, a module standy
>> request must be sent to the module before stopping the module clock.
>>
>> Looks like you're gonna need a whole new RZ/A1-specific clock driver to
>> handle all those details...
>
> There are only a couple IP blocks called out in the STBREQn/STBACKn registers,
> and I think they are not really crucial for runtime pm (Ethenret, LCD,
> Coresight, etc..).
> So in my opinion it's not worth a new driver just yet.

OK.

> But, I think I'd like to disable runtime pm for RZ/A1 in the drivers for now
> Because 'functional' is better than 'lower-power-but-broken'

I prefer not doing that in the individual drivers, as they're shared
with other SoCs.

I think you can handle that in drivers/clk/renesas/clk-mstp.c:
  - in cpg_mstp_attach_dev(), add a call to pm_clk_resume(dev) after the
call to pm_clk_add_clk(),
  - in cpg_mstp_detach_dev(), add a call to pm_clk_suspend(dev) before the
call to pm_clk_destroy().
Yes, that means the module clocks are enabled all the time.
Of course when running on RZ/A1H ;-)

Good luck!

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 v3 2/2] clocksource: Add renesas-ostm timer driver

[Daniel Lezcano]

On Tue, Jan 24, 2017 at 04:45:47AM +, Chris Brandt wrote:

Hi Chris,

[ ... ]

> > > + bool "Renesas OSTM timer driver" if COMPILE_TEST
> > > + depends on GENERIC_CLOCKEVENTS
> > > + select CLKSRC_MMIO
> > > + default SYS_SUPPORTS_RENESAS_OSTM
> > 
> > - default SYS_SUPPORTS_RENESAS_OSTM
> > 
> > Except I missing something, I don' the point of having this intermediate
> > config option.
> 
> I was basically following what all the other clocksource drivers do.
> Selecting an ARCH will then set SYS_SUPPORTS_xxTIMERxx which will then
> force xxTIMERxx=y.

Yeah, little by little these options are clean up to be more consistent across
the different drivers. The sh/mtu drivers are different from the other drivers.

The idea with the config option is to let the platform to select the drivers,
and optionnaly allows the compilation on other architecture to increase the
compilation test coverage. That is the reason of COMPILE_TEST.

> But, if "COMPILE_TEST" is set, you can choose what clocksource timers
> you want to build in.
> 
> Is your suggestion to do away with the COMPILE_TEST ability and
> just force RENESAS_OSTM=y if ARCH_R7S72100 is selected?

Just like that:

config RENESAS_OSTM
bool "Renesas OSTM timer driver" if COMPILE_TEST
depends on GENERIC_CLOCKEVENTS
select CLKSRC_MMIO
help
  Enables the support for the Renesas OSTM

And then from arch/arm/mach-shmobile/Kconfig:

select RENESAS_OSTM


> > > +#include 
> > 
> > Please remove everything module related here and below. This driver is not
> > supposed to be removed and it is compiled in with the Kconfig option.
> 
> Good point. I will remove.
> 
> I guess if you can't compile it as a module, you don't need things like
> 'MODULE_LICENSE'.

Right.

[ ... ]

> > > +static int __init ostm_init_clksrc(struct ostm_device *ostm) {
> > > + int ret;
> > > +
> > > + /* irq not used (clock sources don't use interrupts) */
> > > +
> > > + /* stop counter */
> > 
> > One line comment is usually in the network code. Please use multiline.
> > 
> > /*
> >  * Bla bla
> >  */
> 
> I'm confused. Do you mean:
> 
> A) use multiline formatting for every single-line comment throughout
>the file?
> 
> B) use multiline for any place where you have 2 or more single-line
>comments back to back?

I think it is simpler if you just ignore my comment, remove all your comments
in this function, implement a couple of wrapper (eg. clksrc_stop, clksrc_start)
which will speak by themselves.

By the way, is it normal stopping the clockevent is the same code than stopping
the clocksource ?

[ ... ]

> > > + if (!is_early_platform_device(pdev)) {
> > > + pm_runtime_set_active(>dev);
> > > + pm_runtime_enable(>dev);
> > > + }
> > 
> > Can you clarify why the 'early' is needed here ?
> 
> Actually, it means nothing.
> 
> I was using sh_mtu2.c and sh_cmt.c as reference and they were registering
> an "earlytimer" which made me think the kernel would probe this driver
> early in bootbutnow I see that is just a SH4 kernel specific thing.
> 
> So, I will remove all the early platform reference stuff.
> 
> Of course I still have the question of how are you supposed to get a
> clocksource up and running early in boot. (but I'll figure that out later).

Until the hardware clocksource is registered, the jiffies are used as source of
time. That happens at the very early boot time. The clockevent must be
registered early to update the jiffies but you won't have to care about that if
you use the macro below.

[ ... ]

> > > +early_platform_init("earlytimer", _timer);
> > > +subsys_initcall(ostm_init); module_exit(ostm_exit);
> > > +
> > > +MODULE_AUTHOR("Chris Brandt");
> > > +MODULE_DESCRIPTION("Renesas OSTM Timer Driver"); MODULE_LICENSE("GPL
> > > +v2");
> > 
> > Maybe you can try with builtin_platform ?
> 
> Good idea. But, now I get a "Section mismatch" during link time so I'll
> have to figure out why that is.

Mmh, I think it would be more consistent to convert this to:

CLOCKSOURCE_OF_DECLARE(ostm, "renesas,ostm", ostm_init);

The only problem is to get the struct device associated to the of_node passed
as parameter to ostm_init in order to use the devm_* API.

I think of_find_device_by_node should return the platform_device, then
pdev->dev. If that works the other drivers will benefit from that to remove all
the rollback code everywhere.

  -- Daniel

-- 

  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

RE: [RFC] Runtime PM on the RZ/A series is never going to work right

[Chris Brandt]

Hi Geert,

On Tuesday, January 24, 2017, Geert Uytterhoeven wrote:
> > Therefore, before I start firing off patches to remove runtime PM for
> RZ/A, does anyone have an opinion one way of the other
> 
> Please have a look at Section 55.3.5 ("Module Standby Function"), which I
> had never read myself, apparently...
> 
> Seem like there is some kind of status register, the STBCR register
> itself:
> 
> Canceling Module Standby Function:
> 1. Clear the MSTP bit to 0.
> 2. After that, dummy-read the same register.
> 
> Does it help if you add the dummy read when enabling the clock?

Already tried that. And put a barrier() call just to make sure
everything was done.
It seems that might be enough for the RSPI, but the RIIC still has issues.
From what I can tell, that makes the register space readable...but the IP block
is not fully functional unless you delay a little.


> However, that section reveals another complicating factor for some
> modules:
> if the module has a bit in a STBREQn/STBACKn register, a module standy
> request must be sent to the module before stopping the module clock.
> 
> Looks like you're gonna need a whole new RZ/A1-specific clock driver to
> handle all those details...


There are only a couple IP blocks called out in the STBREQn/STBACKn registers,
and I think they are not really crucial for runtime pm (Ethenret, LCD,
Coresight, etc..).
So in my opinion it's not worth a new driver just yet.


But, I think I'd like to disable runtime pm for RZ/A1 in the drivers for now
Because 'functional' is better than 'lower-power-but-broken'


Chris

Re: [PATCH] arm64: Use __pa_symbol for empty_zero_page

[Mark Rutland]

On Tue, Jan 24, 2017 at 12:43:40PM +0100, Geert Uytterhoeven wrote:
> If CONFIG_DEBUG_VIRTUAL=y and CONFIG_ARM64_SW_TTBR0_PAN=y:
> 
> virt_to_phys used for non-linear address: ff8008cc 
> (empty_zero_page+0x0/0x1000)
> WARNING: CPU: 0 PID: 0 at arch/arm64/mm/physaddr.c:14 
> __virt_to_phys+0x28/0x60
> ...
> [] __virt_to_phys+0x28/0x60
> [] setup_arch+0x46c/0x4d4
> 
> Fixes: 2077be6783b5936c ("arm64: Use __pa_symbol for kernel symbols")
> Signed-off-by: Geert Uytterhoeven 

Acked-by: Mark Rutland 

Mark.

> ---
>  arch/arm64/kernel/setup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 669fc9ff728b227f..b5222094ab52db6f 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -298,7 +298,7 @@ void __init setup_arch(char **cmdline_p)
>* faults in case uaccess_enable() is inadvertently called by the init
>* thread.
>*/
> - init_task.thread_info.ttbr0 = virt_to_phys(empty_zero_page);
> + init_task.thread_info.ttbr0 = __pa_symbol(empty_zero_page);
>  #endif
>  
>  #ifdef CONFIG_VT
> -- 
> 1.9.1
> 

Re: [PATCH] ARM: dts: r7s72100: add power-domains to mmcif

[Simon Horman]

On Mon, Jan 23, 2017 at 04:12:16PM +0100, Geert Uytterhoeven wrote:
> On Mon, Jan 23, 2017 at 3:13 PM, Chris Brandt  
> wrote:
> > Signed-off-by: Chris Brandt 
> 
> Reported-by: Geert Uytterhoeven 
> Acked-by: Geert Uytterhoeven 

Should this be queued up as a fix for v4.10 with the following tag?

Fixes: 887862227ba3 ("ARM: dts: r7s72100: add mmcif to device tree")

Re: [PATCH v3 0/3] ARM: dts: add ostm support for r7s72100

[Simon Horman]

On Mon, Jan 23, 2017 at 08:55:17AM -0500, Chris Brandt wrote:
> This patch set enables the use of the newly created driver
> renesas-ostm.c for the r7s72100 SoC.

Thanks Chris,

I have queued this series up for v4.11.
It should appear in net-next in linux-next day or so.

[PATCH] arm64: Use __pa_symbol for empty_zero_page

[Geert Uytterhoeven]

If CONFIG_DEBUG_VIRTUAL=y and CONFIG_ARM64_SW_TTBR0_PAN=y:

virt_to_phys used for non-linear address: ff8008cc 
(empty_zero_page+0x0/0x1000)
WARNING: CPU: 0 PID: 0 at arch/arm64/mm/physaddr.c:14 
__virt_to_phys+0x28/0x60
...
[] __virt_to_phys+0x28/0x60
[] setup_arch+0x46c/0x4d4

Fixes: 2077be6783b5936c ("arm64: Use __pa_symbol for kernel symbols")
Signed-off-by: Geert Uytterhoeven 
---
 arch/arm64/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 669fc9ff728b227f..b5222094ab52db6f 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -298,7 +298,7 @@ void __init setup_arch(char **cmdline_p)
 * faults in case uaccess_enable() is inadvertently called by the init
 * thread.
 */
-   init_task.thread_info.ttbr0 = virt_to_phys(empty_zero_page);
+   init_task.thread_info.ttbr0 = __pa_symbol(empty_zero_page);
 #endif
 
 #ifdef CONFIG_VT
-- 
1.9.1

Re: [Lager(H2)-V4.10-rc2]: QSPI-FLASH: Warning message happened when writing a data file to SPI-FLASH

[Geert Uytterhoeven]

Hi Cyrille,

On Tue, Jan 24, 2017 at 11:11 AM, Cyrille Pitchen
 wrote:
> Le 24/01/2017 à 08:43, Geert Uytterhoeven a écrit :
>> CC linux-mtd
>>
>> On Tue, Jan 24, 2017 at 7:34 AM, DongCV  wrote:
>>> I've tested the linux v4.10-rc2 for Gen2 Lager and found an issues when
>>> writing data to SPI-FLASH with jffs2 format
>>>
>>> "root@linaro-naro:~# cp test.dat /mnt/media
>>> [  223.874078] [ cut here ]
>>> [  223.888017] WARNING: CPU: 2 PID: 2008 at
>>> drivers/mtd/spi-nor/spi-nor.c:1182 spi_nor_write+0x84/0x160
>>> [  223.915507] Writing at offset 12 into a NOR page. Writing partial pages
>>> may decrease reliability and increase wear of NOR flash.
>>> [  223.915543] CPU: 2 PID: 2008 Comm: cp Not tainted 4.10.0-rc2-dirty #166
>>> [  223.970037] Hardware name: Generic R8A7790 (Flattened Device Tree)
>>> [  223.988563] Backtrace:
>>> [  223.995934] [] (dump_backtrace) from []
>>> (show_stack+0x18/0x1c)
>>> [  224.018639]  r6:c04786f0 r5: r4:600f0013 r3:00404000
>>> [  224.035626] [] (show_stack) from []
>>> (dump_stack+0x80/0xa0)
>>> [  224.057294] [] (dump_stack) from []
>>> (__warn+0xd4/0x104)
>>> [  224.078169]  r5: r4:ef203ba8
>>> [  224.088894] [] (__warn) from []
>>> (warn_slowpath_fmt+0x40/0x48)
>>> [  224.111337]  r9:c0a2a6cb r8:ef203c8c r7:000c r6: r5:
>>> r4:eebfa418
>>> [  224.134564] [] (warn_slowpath_fmt) from []
>>> (spi_nor_write+0x84/0x160)
>>> ..."
>>>
>>> Please have a look at the attached testlog file for detail.
>>
>> I didn't keep the attachment, the test just does
>>
>> # flash_erase -j /dev/mtd2 0 0
>> # mount -t jffs2 /dev/mtdblock2 /mnt/media
>> # dd if=/dev/mtdblock2 of=test.dat bs=512 count=1
>> # cp test.dat /mnt/media
>>
>>> And I found the patch which I think it is the cause of this issues.
>>> The commit name is 'e5d05cb mtd: spi-nor: simplify write loop'
>>> If I reverted this patch and retested it on H2 board, the issue will be
>>> gone!
>
> This is a known issue fixed by the patch "mtd: spi-nor: remove WARN_ONCE()
> message in spi_nor_write()", which is planned to be mainlined in 4.11.
>
> This patch only removes the WARN_ONCE() introduced by commit e5d05cb ("mtd:
> spi-nor: simplify write loop"). All other modifications of commit e5d05cb
> are fine and can be kept. It's only the WARN_ONCE() message that is
> annoying and not so relevant.
>
> So don't worry about this (false) warning and sorry for the noise!

Thanks for explaining!

As this is a user-visible warning, isn't that patch suitable for v4.10?

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/RFC 2/2] iommu/ipmmu-vmsa: Opt-in slave devices based on ES version

[Magnus Damm]

Hi Geert,

On Mon, Jan 23, 2017 at 9:50 PM, Geert Uytterhoeven
 wrote:
> Hi Magnus,
>
> On Mon, Jan 23, 2017 at 1:12 PM, Magnus Damm  wrote:
>> From: Magnus Damm 
>>
>> Match on r8a7795 ES2 and enable a certain DMA controller.
>> In other cases the IPMMU driver remains disabled.
>>
>> Signed-off-by: Magnus Damm 
>> ---
>>
>>  drivers/iommu/ipmmu-vmsa.c |   24 
>>  1 file changed, 24 insertions(+)
>>
>> --- 0001/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-01-23 20:57:02.620607110 +0900
>> @@ -23,6 +23,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
>>  #include 
>> @@ -1002,16 +1003,39 @@ static void ipmmu_domain_free_dma(struct
>> }
>>  }
>>
>> +static const struct soc_device_attribute r8a7795es2[] = {
>> +   { .soc_id = "r8a7795", .revision = "ES2.*" },
>> +   { /* sentinel */ }
>> +};
>> +
>> +static int ipmmu_slave_whitelist(struct device *dev)
>> +{
>> +   /* Opt-in slave devices based on SoC and ES version */
>> +   if (soc_device_match(r8a7795es2)) {
>> +   if (!strcmp(dev_name(dev), "e731.dma-controller"))
>> +   return 0;
>> +   }
>
> I have two comments about the construct above:
>   1. IPMMU will be disabled on all non-r8a7795 SoCs.
>  Is that what you want?

Sort of. This patch is just an example to stir up some discussion
about this topic. I realize this code as-is changes R-Car Gen2
behavior (that is merged upstream) so perhaps we should keep devices
enabled for those SoCs.

>   2. Usually we match on the old broken versions instead (e.g. against
>  "ES1.*"), as (1) it marks more clearly support for old SoCs, and
> (2) it makes it easier to remove the check later when these
> old SoCs are deemed extinct later.

Right, if I understand correctly then you're saying opt-out might be
better instead of opt-in. In case of IPMMU and R-Car Gen3 I believe it
might be less work to use opt-in rather than excluding not-yet-working
stuff. =)

With this series I would like to propose to disconnect the DT
integration timing from the enablement of IPMMU support for slave
devices. If we can enable the IPMMU in DT early on we can reduce
potential out-of-tree IPMMU enablement DT patches. So with the DT
bindings fixed and accurate data sheet we can merge DT bits ahead of
enablement time. And then use run time logic to determine what to
enable based on test results.

As you are aware, currently we have used the presence of "iommus" in
DT to determine if a device is going to be enabled or not. So if the
IPMMU Kconfig bits enable the IPMMU driver and the "iommus" DT
property tie a certain slave device to the IPMMU then we will make use
of IPMMU for a certain device. Currently we assume it will work on all
ES versions that use that particular DTB.

However ES specific hardware errata together with a wide range of ES
versions for r8a7795 and r8a7796 (and whatever SoCs and ES versions
that comes next) makes it difficult to use DT like above to enable
stuff seemingly on one ES version without potentially breaking other
ES versions. I would like to share DT files between the ES versions as
much as possible but still only enable IPMMU support for devices that
are known to work.

Let me know if you think it makes sense to enable DT in a different
way than my proposal.

I'll have a look at putting the white list code in ->xlate() instead
of ->add_device().

Thanks for your comments!

Cheers,

/ magnus

Re: [PATCH v2 2/7] serial: sh-sci: consider DR (data ready) bit adequately

[Geert Uytterhoeven]

On Mon, Jan 23, 2017 at 5:04 PM, Ulrich Hecht
 wrote:
> To allow operation with a higher RX FIFO interrupt threshold in PIO
> mode, it is necessary to consider the DR bit ("FIFO not full, but no
> data received for 1.5 frames") as an indicator that data can be read.
> Otherwise the driver will let data rot in the FIFO until the threshold
> is reached.
>
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

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 4/7] serial: sh-sci: increase RX FIFO trigger defaults for (H)SCIF

[Geert Uytterhoeven]

On Mon, Jan 23, 2017 at 5:04 PM, Ulrich Hecht
 wrote:
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

> @@ -2736,15 +2742,19 @@ static int sci_init_single(struct platform_device 
> *dev,
> sci_port->sampling_rate_mask = SCI_SR_SCIFAB;
> break;
> case PORT_SCIF:
> -   port->fifosize = 16;
> if (p->regtype == SCIx_SH7705_SCIF_REGTYPE) {
> +   port->fifosize = 64;

Note that you forgot to mention that you also corrected the FIFO size for
sh7705, ssh7720, and sh7721. Not that we can easily test that, though.

> sci_port->overrun_reg = SCxSR;
> sci_port->overrun_mask = SCIFA_ORER;
> sci_port->sampling_rate_mask = SCI_SR(16);
> +   /* RX triggering not implemented for this IP */
> +   sci_port->rx_trigger = 1;
> } else {
> +   port->fifosize = 16;
> sci_port->overrun_reg = SCLSR;
> sci_port->overrun_mask = SCLSR_ORER;
> sci_port->sampling_rate_mask = SCI_SR(32);
> +   sci_port->rx_trigger = 8;
> }
> break;
> default:




-- 
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] serial: sh-sci: make RX FIFO parameters tunable via sysfs

[Geert Uytterhoeven]

On Mon, Jan 23, 2017 at 5:04 PM, Ulrich Hecht
 wrote:
> Allows tuning of the RX FIFO fill threshold and timeout. (The latter is
> only applicable to SCIFA and SCIFB).
>
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

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/RFC 1/2] arm64: mm: Silently allow devices lacking IOMMU group

[Sricharan]

Hi Robin,

>> Consider failure of iommu_get_domain_for_dev() as non-critical and
>> get rid of the warning printout. This allows IOMMU properties to be
>> included in the DTB even though the kernel is configured with
>> CONFIG_IOMMU_API=n or in case a particular IOMMU driver refuses to
>> enable IOMMU support for a certain slave device and returns error
>> from the ->add_device() callback.
>>
>> This is only a cosmetic change that removes console warning printouts.
>
>The warning is there for a reason - at this point, we *expected* the
>device to be using an IOMMU for DMA, so a failure is significant. Rather
>than masking genuine failures in other cases because your case
>deliberately breaks that expectation, simply change the expectation -
>i.e. rather than letting of_xlate() succeed then failing add_device()
>later, reject the of_xlate() call up-front such that the DMA layer never
>gets told about the IOMMU in the first place.
>
>Robin.
>

With the iommu probe deferral patches, this behavior would change
where the arch_setup_dma_ops would never be called if there is
an error from xlate or add_device. But also the error value from
xlate/add_device is returned back and the probe of the device
would fail for any error. So if there can be cases like above, where
the xlate/add_device callbacks can return error for specific reasons,
should only EPROBE_DEFER be considered and rest of the errors
be filtered out with a WARN probably ?

Regards,
 Sricharan


>> Signed-off-by: Magnus Damm 
>> ---
>>
>>  arch/arm64/mm/dma-mapping.c |   10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> --- 0001/arch/arm64/mm/dma-mapping.c
>> +++ work/arch/arm64/mm/dma-mapping.c 2017-01-23 20:54:40.060607110 +0900
>> @@ -827,11 +827,19 @@ static bool do_iommu_attach(struct devic
>>  struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>>
>>  /*
>> + * In case IOMMU support is excluded from the kernel or if the device
>> + * is not hooked up to any IOMMU group then be silent and keep the
>> + * old dma_ops.
>> + */
>> +if (!domain)
>> +return false;
>> +
>> +/*
>>   * If the IOMMU driver has the DMA domain support that we require,
>>   * then the IOMMU core will have already configured a group for this
>>   * device, and allocated the default domain for that group.
>>   */
>> -if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
>> +if (iommu_dma_init_domain(domain, dma_base, size, dev)) {
>>  pr_warn("Failed to set up IOMMU for device %s; retaining 
>> platform DMA ops\n",
>>  dev_name(dev));
>>  return false;
>> ___
>> iommu mailing list
>> io...@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
>
>___
>iommu mailing list
>io...@lists.linux-foundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 7/7] arm64: dts: r8a7796: salvator-x: add SCIF1 (DEBUG1)

[Geert Uytterhoeven]

Hi Uli,

On Mon, Jan 23, 2017 at 5:04 PM, Ulrich Hecht
 wrote:
> Enables the SCIF hooked up to the DEBUG1 connector.
>
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

> ---
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts 
> b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> index c7f40f8..498d971 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -18,6 +18,7 @@
>
> aliases {
> serial0 = 
> +   serial1 = 

While the patch is acceptable as-is, my question is still valid:
Is there any specific reason you chose scif1 over hscif1?

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