Devicetree
 help / color / mirror / Atom feed
* Re: [RFC PATCH 14/17] ipmi: kcs: Finish configuring ASPEED KCS device before enable
From: Wang, Haiyue @ 2019-07-26 17:24 UTC (permalink / raw)
  To: Andrew Jeffery, linux-aspeed
  Cc: robh+dt, mark.rutland, joel, devicetree, linux-arm-kernel,
	linux-kernel, Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman,
	openipmi-developer
In-Reply-To: <29a2d999-23bd-8e95-a1b8-f00e25a11df5@linux.intel.com>


在 2019-07-27 01:04, Wang, Haiyue 写道:
> 在 2019-07-26 13:39, Andrew Jeffery 写道:
>> The currently interrupts are configured after the channel was enabled.
>>
>> Cc: Haiyue Wang<haiyue.wang@linux.intel.com>
>> Cc: Corey Minyard<minyard@acm.org>
>> Cc: Arnd Bergmann<arnd@arndb.de>
>> Cc: Greg Kroah-Hartman<gregkh@linuxfoundation.org>
>> Cc:openipmi-developer@lists.sourceforge.net
>> Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
>> ---
>>   drivers/char/ipmi/kcs_bmc_aspeed.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c 
>> b/drivers/char/ipmi/kcs_bmc_aspeed.c
>> index 3c955946e647..e3dd09022589 100644
>> --- a/drivers/char/ipmi/kcs_bmc_aspeed.c
>> +++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
>> @@ -268,13 +268,14 @@ static int aspeed_kcs_probe(struct 
>> platform_device *pdev)
>>       kcs_bmc->io_inputb = aspeed_kcs_inb;
>>       kcs_bmc->io_outputb = aspeed_kcs_outb;
>>   +    rc = aspeed_kcs_config_irq(kcs_bmc, pdev);
>> +    if (rc)
>> +        return rc;
>> +
>>       dev_set_drvdata(dev, kcs_bmc);
>
>
> Thanks for catching this, for not miss the data.
>
Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>

^ permalink raw reply

* Re: [PATCH v3 2/7] drivers: Introduce device lookup variants by of_node
From: Mark Brown @ 2019-07-26 17:18 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Andrew Lunn, Thor Thayer, rafael, Maxime Ripard, linux-fpga,
	dri-devel, linux-kernel, David S. Miller, Srinivas Kandagatla,
	linux-i2c, Frank Rowand, Florian Fainelli, linux-rockchip,
	Lee Jones, Wolfram Sang, David Airlie, Jiri Slaby, devicetree,
	Alan Tull, Liam Girdwood, Rob Herring, Moritz Fischer,
	linux-arm-kernel, Mathieu Poirier, gregkh
In-Reply-To: <20190723221838.12024-3-suzuki.poulose@arm.com>


[-- Attachment #1.1: Type: text/plain, Size: 203 bytes --]

On Tue, Jul 23, 2019 at 11:18:33PM +0100, Suzuki K Poulose wrote:
> Introduce wrappers for {bus/driver/class}_find_device() to
> locate devices by its of_node.

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [RFC PATCH 14/17] ipmi: kcs: Finish configuring ASPEED KCS device before enable
From: Wang, Haiyue @ 2019-07-26 17:04 UTC (permalink / raw)
  To: Andrew Jeffery, linux-aspeed
  Cc: robh+dt, mark.rutland, joel, devicetree, linux-arm-kernel,
	linux-kernel, Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman,
	openipmi-developer
In-Reply-To: <20190726053959.2003-15-andrew@aj.id.au>

在 2019-07-26 13:39, Andrew Jeffery 写道:
> The currently interrupts are configured after the channel was enabled.
>
> Cc: Haiyue Wang<haiyue.wang@linux.intel.com>
> Cc: Corey Minyard<minyard@acm.org>
> Cc: Arnd Bergmann<arnd@arndb.de>
> Cc: Greg Kroah-Hartman<gregkh@linuxfoundation.org>
> Cc:openipmi-developer@lists.sourceforge.net
> Signed-off-by: Andrew Jeffery<andrew@aj.id.au>
> ---
>   drivers/char/ipmi/kcs_bmc_aspeed.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
> index 3c955946e647..e3dd09022589 100644
> --- a/drivers/char/ipmi/kcs_bmc_aspeed.c
> +++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
> @@ -268,13 +268,14 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
>   	kcs_bmc->io_inputb = aspeed_kcs_inb;
>   	kcs_bmc->io_outputb = aspeed_kcs_outb;
>   
> +	rc = aspeed_kcs_config_irq(kcs_bmc, pdev);
> +	if (rc)
> +		return rc;
> +
>   	dev_set_drvdata(dev, kcs_bmc);


Thanks for catching this, for not miss the data.

^ permalink raw reply

* Re: [PATCH v3 2/4] edac: Add support for Amazon's Annapurna Labs L1 EDAC
From: James Morse @ 2019-07-26 16:49 UTC (permalink / raw)
  To: Hanna Hawa
  Cc: robh+dt, mark.rutland, bp, mchehab, davem, gregkh, linus.walleij,
	Jonathan.Cameron, nicolas.ferre, paulmck, dwmw, benh, ronenk,
	talel, jonnyc, hanochu, devicetree, linux-kernel, linux-edac
In-Reply-To: <1563197049-12679-3-git-send-email-hhhawa@amazon.com>

Hi Hanna,

On 15/07/2019 14:24, Hanna Hawa wrote:
> Adds support for Amazon's Annapurna Labs L1 EDAC driver to detect and
> report L1 errors.

> diff --git a/drivers/edac/al_l1_edac.c b/drivers/edac/al_l1_edac.c
> new file mode 100644
> index 0000000..70510ea
> --- /dev/null
> +++ b/drivers/edac/al_l1_edac.c
> @@ -0,0 +1,156 @@

> +#include <linux/bitfield.h>

You need <linux/smp.h> for on-each_cpu().

> +#include "edac_device.h"
> +#include "edac_module.h"

You need <asm/sysreg.h> for the sys_reg() macro. The ARCH_ALPINE dependency doesn't stop
this from being built on 32bit arm, where this sys_reg() won't work/exist.

[...]

> +static void al_l1_edac_cpumerrsr(void *arg)
> +{
> +	struct edac_device_ctl_info *edac_dev = arg;
> +	int cpu, i;
> +	u32 ramid, repeat, other, fatal;
> +	u64 val = read_sysreg_s(ARM_CA57_CPUMERRSR_EL1);
> +	char msg[AL_L1_EDAC_MSG_MAX];
> +	int space, count;
> +	char *p;
> +	if (!(FIELD_GET(ARM_CA57_CPUMERRSR_VALID, val)))
> +		return;
> +	space = sizeof(msg);
> +	p = msg;
> +	count = snprintf(p, space, "CPU%d L1 %serror detected", cpu,
> +			 (fatal) ? "Fatal " : "");
> +	p += count;
> +	space -= count;

snprintf() will return the number of characters it would have generated, even if that is
more than space. If this happen, space becomes negative, p points outside msg[] and msg[]
isn't NULL terminated...

It looks like you want scnprintf(), which returns the number of characters written to buf
instead. (I don't see how 256 characters would be printed by this code)


> +	switch (ramid) {
> +	case ARM_CA57_L1_I_TAG_RAM:
> +		count = snprintf(p, space, " RAMID='L1-I Tag RAM'");
> +		break;
> +	case ARM_CA57_L1_I_DATA_RAM:
> +		count = snprintf(p, space, " RAMID='L1-I Data RAM'");
> +		break;
> +	case ARM_CA57_L1_D_TAG_RAM:
> +		count = snprintf(p, space, " RAMID='L1-D Tag RAM'");
> +		break;
> +	case ARM_CA57_L1_D_DATA_RAM:
> +		count = snprintf(p, space, " RAMID='L1-D Data RAM'");
> +		break;
> +	case ARM_CA57_L2_TLB_RAM:
> +		count = snprintf(p, space, " RAMID='L2 TLB RAM'");
> +		break;
> +	default:
> +		count = snprintf(p, space, " RAMID='unknown'");
> +		break;
> +	}
> +
> +	p += count;
> +	space -= count;
> +	count = snprintf(p, space,
> +			 " repeat=%d, other=%d (CPUMERRSR_EL1=0x%llx)",
> +			 repeat, other, val);
> +
> +	for (i = 0; i < repeat; i++) {
> +		if (fatal)
> +			edac_device_handle_ue(edac_dev, 0, 0, msg);
> +		else
> +			edac_device_handle_ce(edac_dev, 0, 0, msg);
> +	}
> +
> +	write_sysreg_s(0, ARM_CA57_CPUMERRSR_EL1);

Writing 0 just after you've read the value would minimise the window where repeat could
have increased behind your back, or another error was counted as other, when it could have
been reported more accurately.


> +}


> +static int al_l1_edac_probe(struct platform_device *pdev)
> +{
> +	struct edac_device_ctl_info *edac_dev;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	edac_dev = edac_device_alloc_ctl_info(0, (char *)dev_name(dev), 1, "L",
> +					      1, 1, NULL, 0,
> +					      edac_device_alloc_index());
> +	if (IS_ERR(edac_dev))

edac_device_alloc_ctl_info() returns NULL, or dev_ctl, which comes from kzalloc(). I think
you need to check for NULL here, IS_ERR() only catches the -errno range. (there is an
IS_ERR_OR_NULL() if you really needed both)


> +		return -ENOMEM;


With the header-includes and edac_device_alloc_ctl_info() NULL check:
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James

^ permalink raw reply

* Re: [PATCH v3 6a/7] dt-bindings: Add ANX6345 DP/eDP transmitter binding
From: Maxime Ripard @ 2019-07-26 16:36 UTC (permalink / raw)
  To: Torsten Duwe
  Cc: Mark Rutland, devicetree, David Airlie, Greg Kroah-Hartman,
	linux-kernel, dri-devel, Vasily Khoruzhick, Chen-Yu Tsai,
	Rob Herring, Thierry Reding, Laurent Pinchart, Harald Geyer,
	Sean Paul, Thomas Gleixner, linux-arm-kernel, Icenowy Zheng
In-Reply-To: <20190725151829.DC20968B02@verein.lst.de>

Hi,

On Thu, Jul 25, 2019 at 05:18:29PM +0200, Torsten Duwe wrote:
> The anx6345 is an ultra-low power DisplayPort/eDP transmitter designed
> for portable devices.
>
> Add a binding document for it.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/display/bridge/anx6345.yaml |   90 ++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/anx6345.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/anx6345.yaml b/Documentation/devicetree/bindings/display/bridge/anx6345.yaml
> new file mode 100644
> index 000000000000..0af092d101c5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/anx6345.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/anx6345.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analogix ANX6345 eDP Transmitter Device Tree Bindings
> +
> +maintainers:
> +  - Torsten Duwe <duwe@lst.de>
> +
> +description: |
> +  The ANX6345 is an ultra-low power Full-HD eDP transmitter designed for
> +  portable devices.
> +
> +properties:
> +  compatible:
> +    const: analogix,anx6345
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C address of the device
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: active low GPIO to use for reset
> +
> +  dvdd12-supply:
> +    maxItems: 1
> +    description: Regulator for 1.2V digital core power.
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  dvdd25-supply:
> +    maxItems: 1
> +    description: Regulator for 2.5V digital core power.
> +    $ref: /schemas/types.yaml#/definitions/phandle

There's no need to specify the type here, all the properties ending in
-supply are already checked for that type

> +  ports:
> +    type: object
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      Video port 0 for LVTTL input,
> +      Video port 1 for eDP output (panel or connector)
> +      using the DT bindings defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt

You should probably describe the port@0 and port@1 nodes here as
well. It would allow you to express that the port 0 is mandatory and
the port 1 optional, which got dropped in the conversion.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] ARM: dts: stm32: fix -Wall W=1 compilation warnings for can1_sleep pinctrl
From: Alexandre Torgue @ 2019-07-26 16:27 UTC (permalink / raw)
  To: Erwan Le Ray, Maxime Coquelin, Rob Herring, Mark Rutland
  Cc: devicetree, bich.hemon, linux-stm32, linux-arm-kernel,
	linux-kernel
In-Reply-To: <1561972686-23281-1-git-send-email-erwan.leray@st.com>

Hi Erwan		

On 7/1/19 11:18 AM, Erwan Le Ray wrote:
> Fix compilations warnings detected by -Wall W=1 compilation option:
> - node has a unit name, but no reg property
> 
> Signed-off-by: Erwan Le Ray <erwan.leray@st.com>
> 
> diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> index 140a983..ce98fd8 100644
> --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
> @@ -427,7 +427,7 @@
>   				};
>   			};
>   
> -			m_can1_sleep_pins_a: m_can1-sleep@0 {
> +			m_can1_sleep_pins_a: m_can1-sleep-0 {
>   				pins {
>   					pinmux = <STM32_PINMUX('H', 13, ANALOG)>, /* CAN1_TX */
>   						 <STM32_PINMUX('I', 9, ANALOG)>; /* CAN1_RX */
> 

Thanks for cleaning the STM32 DT. Applied on stm32-next. Note that I 
changed commit title to indicate which STM32 platform is targeted by 
this patch.

Regards
Alex

^ permalink raw reply

* Re: [PATCH v3 6/6] interconnect: Add OPP table support for interconnects
From: Georgi Djakov @ 2019-07-26 16:25 UTC (permalink / raw)
  To: Saravana Kannan, Rob Herring, Mark Rutland, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
  Cc: vincent.guittot, seansw, daidavid1, Rajendra Nayak, sibis,
	bjorn.andersson, evgreen, kernel-team, linux-pm, devicetree,
	linux-kernel
In-Reply-To: <20190703011020.151615-7-saravanak@google.com>

Hi Saravana,

On 7/3/19 04:10, Saravana Kannan wrote:
> Interconnect paths can have different performance points. Now that OPP
> framework supports bandwidth OPP tables, add OPP table support for
> interconnects.
> 
> Devices can use the interconnect-opp-table DT property to specify OPP
> tables for interconnect paths. And the driver can obtain the OPP table for
> an interconnect path by calling icc_get_opp_table().
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  drivers/interconnect/core.c  | 27 ++++++++++++++++++++++++++-
>  include/linux/interconnect.h |  7 +++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 871eb4bc4efc..881bac80bc1e 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -47,6 +47,7 @@ struct icc_req {
>   */
>  struct icc_path {
>  	size_t num_nodes;
> +	struct opp_table *opp_table;

I am a bit worried that these tables might be abused and size of the DT will
grow with many OPP tables of all existing paths.

>  	struct icc_req reqs[];
>  };
>  
> @@ -313,7 +314,7 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
>  {
>  	struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
>  	struct icc_node *src_node, *dst_node;
> -	struct device_node *np = NULL;
> +	struct device_node *np = NULL, *opp_node;
>  	struct of_phandle_args src_args, dst_args;
>  	int idx = 0;
>  	int ret;
> @@ -381,10 +382,34 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
>  		dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
>  	mutex_unlock(&icc_lock);
>  
> +	opp_node = of_parse_phandle(np, "interconnect-opp-table", idx);

Can't we figure out if the device OPP table contains bandwidth even without this
property?

Thanks,
Georgi

> +	if (opp_node) {
> +		path->opp_table = dev_pm_opp_of_find_table_from_node(opp_node);
> +		of_node_put(opp_node);
> +	}
> +
> +
>  	return path;
>  }
>  EXPORT_SYMBOL_GPL(of_icc_get);
>  
> +/**
> + * icc_get_opp_table() - Get the OPP table that corresponds to a path
> + * @path: reference to the path returned by icc_get()
> + *
> + * This function will return the OPP table that corresponds to a path handle.
> + * If the interconnect API is disabled, NULL is returned and the consumer
> + * drivers will still build. Drivers are free to handle this specifically, but
> + * they don't have to.
> + *
> + * Return: opp_table pointer on success. NULL is returned when the API is
> + * disabled or the OPP table is missing.
> + */
> +struct opp_table *icc_get_opp_table(struct icc_path *path)
> +{
> +	return path->opp_table;
> +}
> +
>  /**
>   * icc_set_bw() - set bandwidth constraints on an interconnect path
>   * @path: reference to the path returned by icc_get()
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> index dc25864755ba..0c0bc55f0e89 100644
> --- a/include/linux/interconnect.h
> +++ b/include/linux/interconnect.h
> @@ -9,6 +9,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/types.h>
> +#include <linux/pm_opp.h>
>  
>  /* macros for converting to icc units */
>  #define Bps_to_icc(x)	((x) / 1000)
> @@ -28,6 +29,7 @@ struct device;
>  struct icc_path *icc_get(struct device *dev, const int src_id,
>  			 const int dst_id);
>  struct icc_path *of_icc_get(struct device *dev, const char *name);
> +struct opp_table *icc_get_opp_table(struct icc_path *path);
>  void icc_put(struct icc_path *path);
>  int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
>  
> @@ -49,6 +51,11 @@ static inline void icc_put(struct icc_path *path)
>  {
>  }
>  
> +static inline struct opp_table *icc_get_opp_table(struct icc_path *path)
> +{
> +	return NULL;
> +}
> +
>  static inline int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
>  {
>  	return 0;
> 

^ permalink raw reply

* Re: [PATCH v3 1/6] dt-bindings: opp: Introduce opp-peak-KBps and opp-avg-KBps bindings
From: Georgi Djakov @ 2019-07-26 16:24 UTC (permalink / raw)
  To: Saravana Kannan, Rob Herring, Mark Rutland, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael J. Wysocki
  Cc: vincent.guittot, seansw, daidavid1, Rajendra Nayak, sibis,
	bjorn.andersson, evgreen, kernel-team, linux-pm, devicetree,
	linux-kernel
In-Reply-To: <20190703011020.151615-2-saravanak@google.com>

Hi Saravana,

On 7/3/19 04:10, Saravana Kannan wrote:
> Interconnects often quantify their performance points in terms of
> bandwidth. So, add opp-peak-KBps (required) and opp-avg-KBps (optional) to
> allow specifying Bandwidth OPP tables in DT.
> 
> opp-peak-KBps is a required property that replace opp-hz for Bandwidth OPP
> tables.
> 
> opp-avg-KBps is an optional property that can be used in Bandwidth OPP
> tables.
> 
> Signed-off-by: Saravana Kannan <saravanak@google.com>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 76b6c79604a5..c869e87caa2a 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -83,9 +83,14 @@ properties.
>  
>  Required properties:
>  - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
> -  required property for all device nodes but devices like power domains. The
> -  power domain nodes must have another (implementation dependent) property which
> -  uniquely identifies the OPP nodes.
> +  required property for all device nodes but for devices like power domains or
> +  bandwidth opp tables. The power domain nodes must have another (implementation
> +  dependent) property which uniquely identifies the OPP nodes. The interconnect
> +  opps are required to have the opp-peak-bw property.
> +
> +- opp-peak-KBps: Peak bandwidth in kilobytes per second, expressed as a 32-bit

As Rob already mentioned, KBps should be documented. See [1].

Thanks,
Georgi

[1] https://lore.kernel.org/lkml/20190423132823.7915-2-georgi.djakov@linaro.org/

^ permalink raw reply

* Re: [PATCH net-next 3/3] dt-bindings: net: ethernet: Update mt7622 docs and dts to reflect the new phylink API
From: Russell King - ARM Linux admin @ 2019-07-26 16:23 UTC (permalink / raw)
  To: René van Dorst
  Cc: Andrew Lunn, netdev, frank-w, sean.wang, f.fainelli, davem,
	matthias.bgg, vivien.didelot, john, linux-mediatek, linux-mips,
	robh+dt, devicetree
In-Reply-To: <20190726151622.Horde.1AA717IbQrC7_YJcSBe4M-0@www.vdorst.com>

On Fri, Jul 26, 2019 at 03:16:22PM +0000, René van Dorst wrote:
> Quoting Andrew Lunn <andrew@lunn.ch>:
> > On Fri, Jul 26, 2019 at 07:19:56AM +0000, René van Dorst wrote:
> > > Quoting Andrew Lunn <andrew@lunn.ch>:
> > > 
> > > >>+	gmac0: mac@0 {
> > > >>+		compatible = "mediatek,eth-mac";
> > > >>+		reg = <0>;
> > > >>+		phy-mode = "sgmii";
> > > >>+
> > > >>+		fixed-link {
> > > >>+			speed = <2500>;
> > > >>+			full-duplex;
> > > >>+			pause;
> > > >>+		};
> > > >>+	};
> > > >
> > > >Hi René
> > > >
> > > 
> > > Hi Andrew,
> > > 
> > > >SGMII and fixed-link is rather odd. Why do you need this combination?
> > > 
> > > BananaPi R64 has a RTL8367S 5+2-port switch, switch interfaces with the SOC
> > > by a
> > > (H)SGMII and/or RGMII interface. SGMII is mainly used for the LAN ports and
> > > RGMII for the WAN port.
> > > 
> > > I mimic the SDK software which puts SGMII interface in 2.5GBit fixed-link
> > > mode.
> > > The RTL8367S switch code also put switch mac in forge 2.5GBit mode.
> > > 
> > > So this is the reason why I put a fixed-link mode here.
> > 
> > Are you sure it is using SGMII and not 2500BaseX? Can you get access
> > to the signalling word? SGMII is supposed to indicate to the MAC what
> > speed it is using, via inband signalling. So there should not be any
> > need for a fixed-link. 2500BaseX however does not have such
> > signalling, so there would need to be a fixed link.
> 
> I am not sure.
> 
> I just converted the current mainline code to support phylink and mimic the
> DTS
> of the SDK. But the SDK seems to be incorrect.
> 
> Realtek[0] calls these modes:
> * SGMII (1.25GHz) Interface
> * High SGMII (3.125GHz) Interface
> Also the datasheet that I have doesn't talk about base-x modes.

So this is RTL8367S-CG, which is a switch.  It's entirely possible that
it really does support what it says it does.

> But MT7622 Reference manual[1] page 1960 says:
>  The core leverages the 1000Base-X PCS and Auto-Negotiation from IEEE 802.3
>  specification (clause 36/37). This IP can support up to 3.125G baud for
> 2.5Gbps
>  (proprietary 2500Base-X) data rate of MAC by overclocking.
> 
> So I think it phy-mode should be 2500Base-X in this case.

Right, so this suggests that it only supports 1000BASE-X and 2500BASE-X
via the normal method of "over-clocking" 1000BASE-X.

1000BASE-X and SGMII are compatible _if_ and only if you ignore the
contents of the 16-bit control word which is used for auto-negotiation
in the case of 1000BASE-X, or for communicating the negotiation results
in the case of SGMII.  Apart from that 16-bit control word, and the
semantics of it, at the data link level the two are the same.

> SGMII part is a bit hard for me to support, I don't have the hardware,
> MediaTek datasheets are mostly incomplete and also I am a not familiar with
> it.
> 
> But I think I know what I have to change.
> Based on your explanation above.
> 
> I think this more correct implementation:
> 
> * 1000base-x and 2500base-x always force the link.

I think the above is why you have to force the link: a link consisting
of one end configured for SGMII and the other end configured for
1000BASE-X is not a good idea at the best of times, but if you ignore
the 16-bit control word and force it, it will work.

What this means is that you _should_ be forcing it in DT to be a
fixed link, and not trying to do auto-neg.

> * SGMII is always inband but I need in phylink_mac_link_status() to readout
>   "PCS_SPEED_ABILITY Clause 45 3.5" register to see the inband status?
>   Or is it just the GMAC PSMR register? For me it is a bit confusing.
>   SGMII block has a register to set the link speed and etc. But tests on the
>   bananapi R64 board shows that I also need to set the GMAC register else it
>   didn't work. Also it is not easy to debug if you don't have the board.

phylink_mac_link_status() is expected to read the results of SGMII
or 1000BASE-X negotiation at the MAC side of the link.  To see why,
consider a fiber link:

MAC-PCS --- SFP ----- fiber ----- SFP --- MAC-PCS

The fiber is passive, the SFP merely converts between light and
electrical signals - there's nothing apart from the MAC's own PCS
that can report what the negotiation state of the link is.  So,
you need to read from whatever bit of hardware on the MAC side
which will report that - basically, the results of the 1000BASE-X
negotiation.

phylink currently expects results from the PCS to be automatically
propagated to the MAC through hardware, since that's what happens
on Marvell setups - however, that can be changed if there are
setups which need manual propagation.

If we do need to do that, I'd suggest we rename
phylink_mac_link_status() to be phylink_macpcs_link_status() to
clarify which bit of hardware it's supposed to be reading from.

> 
> > Maybe we should really consider what phy-mode = "sgmii"; means. Should
> > this include the overclocked 2.5G speed, or should we add a 2500sgmii
> > link mode?
> 
> No.

I'm really not in favour of "sgmii" being used to also describe the
over-clocked SGMII variants.  It's a different protocol - many data
sheets (e.g. for PHYs that support it) explicitly state that the
speed bits in the SGMII 16-bit control word are not valid, and
over-clocked vs normal SGMII can not be auto-negotiated.  Both ends
must be running at the same speed in order to successfully transfer
even the 16-bit control word that dictates the link speed.

So, SGMII at 3.125Gbps is really a different interface mode from
SGMII at 1.25Gbps.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* [PATCH V3 1/3] dt-bindings: net: dsa: ksz: document Microchip KSZ87xx family switches
From: Marek Vasut @ 2019-07-26 16:23 UTC (permalink / raw)
  To: netdev
  Cc: Marek Vasut, Andrew Lunn, David S . Miller, Florian Fainelli,
	Rob Herring, Tristram Ha, Vivien Didelot, Woojung Huh, devicetree
In-Reply-To: <20190726162308.16764-1-marex@denx.de>

Document Microchip KSZ87xx family switches. These include
KSZ8765 - 5 port switch
KSZ8794 - 4 port switch
KSZ8795 - 5 port switch

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Vivien Didelot <vivien.didelot@gmail.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
Cc: devicetree@vger.kernel.org
---
V2: No change
V3: No change
---
 Documentation/devicetree/bindings/net/dsa/ksz.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 4ac21cef370e..5e8429b6f9ca 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -5,6 +5,9 @@ Required properties:
 
 - compatible: For external switch chips, compatible string must be exactly one
   of the following:
+  - "microchip,ksz8765"
+  - "microchip,ksz8794"
+  - "microchip,ksz8795"
   - "microchip,ksz9477"
   - "microchip,ksz9897"
   - "microchip,ksz9896"
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH v2 0/4] regulator: add support for the STM32 ADC booster
From: Alexandre Torgue @ 2019-07-26 16:22 UTC (permalink / raw)
  To: Fabrice Gasnier, broonie, lgirdwood, robh+dt
  Cc: devicetree, linux-stm32, linux-arm-kernel, mcoquelin.stm32,
	linux-kernel
In-Reply-To: <1561968865-22037-1-git-send-email-fabrice.gasnier@st.com>

Hi Fabrice

On 7/1/19 10:14 AM, Fabrice Gasnier wrote:
> Add support for the 3.3V booster regulator embedded in stm32h7 and stm32mp1
> devices, that can be used to supply ADC analog input switches.
> It's useful to reach full ADC performance when their supply is below 2.7V
> (vdda by default).
> 
> Changes in v2:
> - rebase on top of for-next branch
> 
> Fabrice Gasnier (4):
>    dt-bindings: regulator: add support for the stm32-booster
>    regulator: add support for the stm32-booster
>    ARM: multi_v7_defconfig: enable STM32 booster regulator
>    ARM: dts: stm32: add booster for ADC analog switches on stm32mp157c
> 
>   .../bindings/regulator/st,stm32-booster.txt        |  18 +++
>   arch/arm/boot/dts/stm32mp157c.dtsi                 |   6 +
>   arch/arm/configs/multi_v7_defconfig                |   1 +
>   drivers/regulator/Kconfig                          |  11 ++
>   drivers/regulator/Makefile                         |   1 +
>   drivers/regulator/stm32-booster.c                  | 132 +++++++++++++++++++++
>   6 files changed, 169 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/regulator/st,stm32-booster.txt
>   create mode 100644 drivers/regulator/stm32-booster.c
> 

DT patch + config applied in stm32-next.

regards
alex

^ permalink raw reply

* Re: [PATCH v4 0/8] stm32 m4 remoteproc on STM32MP157c
From: Alexandre Torgue @ 2019-07-26 16:18 UTC (permalink / raw)
  To: Fabien Dessenne, Rob Herring, Mark Rutland, Maxime Coquelin,
	Ohad Ben-Cohen, Bjorn Andersson, devicetree, linux-stm32,
	linux-arm-kernel, linux-kernel, linux-remoteproc
  Cc: Arnaud Pouliquen, Ludovic Barre, Loic Pallardy, Benjamin Gaignard
In-Reply-To: <1557822423-22658-1-git-send-email-fabien.dessenne@st.com>

Hi Fabien

On 5/14/19 10:26 AM, Fabien Dessenne wrote:
> STMicrolectronics STM32MP157 MPU are based on a Dual Arm Cortex-A7 core and a
> Cortex-M4.
> This patchset adds the support of the stm32_rproc driver allowing to control
> the M4 remote processor.
> 

...

>      driver
>    remoteproc: stm32: add an ST stm32_rproc driver
>    ARM: dts: stm32: add m4 remoteproc support on STM32MP157c
>    ARM: dts: stm32: declare copro reserved memories on STM32MP157c-ed1
>    ARM: dts: stm32: enable m4 coprocessor support on STM32MP157c-ed1
>    ARM: dts: stm32: declare copro reserved memories on STM32MP157a-dk1
>    ARM: dts: stm32: enable m4 coprocessor support on STM32MP157a-dk1
> 

DT patches applied on stm32-next.

Regards
Alexandre

^ permalink raw reply

* Re: Controllers with several interface options - one or more drivers?
From: Sam Ravnborg @ 2019-07-26 16:14 UTC (permalink / raw)
  To: Daniel Vetter, Noralf Trønnes
  Cc: devicetree, Dave Airlie, Josef Lusticky, dri-devel,
	Thierry Reding, Laurent Pinchart
In-Reply-To: <CAKMK7uESP5D4e_Qx6W7amURqxJ=5Y4JHduZYCtkyVQY9jKJQeA@mail.gmail.com>

Hi Daniel.

Added Noralf - somehow I missed him on the original mail.

On Fri, Jul 26, 2019 at 05:06:03PM +0200, Daniel Vetter wrote:
> Also probably should add a few more (drm_bridge) people, I think
> that's also somewhat relevant here.
> -Daniel
> 
> On Fri, Jul 26, 2019 at 4:55 PM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Fri, Jul 26, 2019 at 02:25:10PM +0200, Sam Ravnborg wrote:
> > > Hi Josef, Daniel et al.
> > >
> > > The driver that triggered this reply is a driver that adds parallel
> > > support to ili9341 in a dedicated panel driver.
> > > The issue here is that we already have a tiny driver that supports the
> > > ili9341 controller - but with a slightly different configuration.
> > >
> > > The ili9341 supports several interfaces - from the datasheet:
> > >     "ILI9341 supports parallel 8-/9-/16-/18-bit data bus
> > >      MCU interface, 6-/16-/18-bit data bus RGB interface and
> > >      3-/4-line serial peripheral interface (SPI)"
> > >
> > > Noralf - in another mail explained:
> > > "
> > > The MIPI Alliance has lots of standards some wrt. display controller
> > > interfaces:
> > > - MIPI DBI - Display Bus Interface (used for commands and optionally pixels)
> > > - MIPI DPI - Display Pixel Interface (also called RGB interface or
> > > DOTCLK interface)
> > > - MIPI DSI - Display Serial Interface (commands and pixels)
> > >
> > > The ili9341 supports both MIPI DBI and DPI.
> > > "
> > >
> > > MIPI DPI - is a good fit for a drm_panel driver.
> > > MIPI DBI - requires a full display controller driver.
> > >
> > > There are many other examples of driver SoC that in the same way
> > > can be seen only as a panel or as a full display controller driver.
> > >
> > > The open question here is if we should try to support both cases in the
> > > same driver / file. Or shall we implment two different drivers.
> > > One for the panel use-case. And one for the display controller usecase?
> > >
> > > Not sure - so asking for feedback.
> >
> > I'm not sure. Currently we do have DSI and dumb RGB panels all in
> > drm/panel. I don't think we have DBI panels in there yet, but then
> > drm/tiny is the only one supporting these.
> >
> > I guess we could look into move some of the DBI panel drivers into panel
> > drivers, but that needs a bit more glue all around. I'm honestly not sure
> > how the current DSI drivers in drm_panel work exactly, especially for
> > command mode.
> >
> > Or maybe we need a new interface for command mode.
If I get around to do a driver for the ssd1306 then I will try to sewhat
makes sense then. For now we shall not stall the ili9341 driver.
> >
> > Wrt sharing code between drivers for the same chip, but different
> > interfaces: I wouldn't worry too much about that. Maybe try to have a
> > shared header file at least for registers.
This part should be the minimum. Somthing like include/drm/mipi/?

	Sam

> > Long term we could end up with
> > one driver module which exposes different flavours of the same chip, so
> > multiple drm_panel drivers, or maybe we'll get something more specific for
> > dsi/dbi.
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 0/5] Add missing vdda-supply to STM32 ADC
From: Alexandre Torgue @ 2019-07-26 16:00 UTC (permalink / raw)
  To: Fabrice Gasnier, jic23, robh+dt
  Cc: mark.rutland, devicetree, lars, mcoquelin.stm32, linux-iio,
	linux-kernel, pmeerw, knaack.h, linux-stm32, linux-arm-kernel
In-Reply-To: <1560947398-11592-1-git-send-email-fabrice.gasnier@st.com>

Hi Fabrice

On 6/19/19 2:29 PM, Fabrice Gasnier wrote:
> Add missing vdda-supply, analog power supply, to STM32 ADC. When vdda is
> an independent supply, it needs to be properly turned on or off to supply
> the ADC.
> This series proposes fixes for the dt-bindings, IIO driver and relevant
> device tree files.
> 
> Fabrice Gasnier (5):
>    dt-bindings: iio: adc: stm32: add missing vdda supply
>    iio: adc: stm32-adc: add missing vdda-supply
>    ARM: dts: stm32: remove fixed regulator unit address on stm32429i-eval
>    ARM: dts: stm32: add missing vdda-supply to adc on stm32429i-eval
>    ARM: dts: stm32: add missing vdda-supply to adc on stm32h743i-eval
> 
>   .../devicetree/bindings/iio/adc/st,stm32-adc.txt   |  1 +
>   arch/arm/boot/dts/stm32429i-eval.dts               | 25 +++++++++++-----------
>   arch/arm/boot/dts/stm32h743i-eval.dts              |  1 +
>   drivers/iio/adc/stm32-adc-core.c                   | 21 +++++++++++++++++-
>   4 files changed, 35 insertions(+), 13 deletions(-)
> 

DT patches applied on stm32-next. I plan to add them in my PR for v5.4.
However those patches are marked as "fixes", do you see an issue to only 
send it for v5.4 ?

Regards
alex

^ permalink raw reply

* Re: [PATCH net-next 3/3] dt-bindings: net: ethernet: Update mt7622 docs and dts to reflect the new phylink API
From: René van Dorst @ 2019-07-26 15:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, frank-w, sean.wang, f.fainelli, linux, davem,
	matthias.bgg, vivien.didelot, john, linux-mediatek, linux-mips,
	robh+dt, devicetree
In-Reply-To: <20190726131604.GA18223@lunn.ch>

Quoting Andrew Lunn <andrew@lunn.ch>:

> On Fri, Jul 26, 2019 at 07:19:56AM +0000, René van Dorst wrote:
>> Quoting Andrew Lunn <andrew@lunn.ch>:
>>
>> >>+	gmac0: mac@0 {
>> >>+		compatible = "mediatek,eth-mac";
>> >>+		reg = <0>;
>> >>+		phy-mode = "sgmii";
>> >>+
>> >>+		fixed-link {
>> >>+			speed = <2500>;
>> >>+			full-duplex;
>> >>+			pause;
>> >>+		};
>> >>+	};
>> >
>> >Hi René
>> >
>>
>> Hi Andrew,
>>
>> >SGMII and fixed-link is rather odd. Why do you need this combination?
>>
>> BananaPi R64 has a RTL8367S 5+2-port switch, switch interfaces with the SOC
>> by a
>> (H)SGMII and/or RGMII interface. SGMII is mainly used for the LAN ports and
>> RGMII for the WAN port.
>>
>> I mimic the SDK software which puts SGMII interface in 2.5GBit fixed-link
>> mode.
>> The RTL8367S switch code also put switch mac in forge 2.5GBit mode.
>>
>> So this is the reason why I put a fixed-link mode here.
>
> Are you sure it is using SGMII and not 2500BaseX? Can you get access
> to the signalling word? SGMII is supposed to indicate to the MAC what
> speed it is using, via inband signalling. So there should not be any
> need for a fixed-link. 2500BaseX however does not have such
> signalling, so there would need to be a fixed link.

I am not sure.

I just converted the current mainline code to support phylink and  
mimic the DTS
of the SDK. But the SDK seems to be incorrect.

Realtek[0] calls these modes:
* SGMII (1.25GHz) Interface
* High SGMII (3.125GHz) Interface
Also the datasheet that I have doesn't talk about base-x modes.

But MT7622 Reference manual[1] page 1960 says:
  The core leverages the 1000Base-X PCS and Auto-Negotiation from IEEE 802.3
  specification (clause 36/37). This IP can support up to 3.125G baud  
for 2.5Gbps
  (proprietary 2500Base-X) data rate of MAC by overclocking.

So I think it phy-mode should be 2500Base-X in this case.

SGMII part is a bit hard for me to support, I don't have the hardware,
MediaTek datasheets are mostly incomplete and also I am a not familiar  
with it.

But I think I know what I have to change.
Based on your explanation above.

I think this more correct implementation:

* 1000base-x and 2500base-x always force the link.
* SGMII is always inband but I need in phylink_mac_link_status() to readout
   "PCS_SPEED_ABILITY Clause 45 3.5" register to see the inband status?
   Or is it just the GMAC PSMR register? For me it is a bit confusing.
   SGMII block has a register to set the link speed and etc. But tests on the
   bananapi R64 board shows that I also need to set the GMAC register else it
   didn't work. Also it is not easy to debug if you don't have the board.

> Maybe we should really consider what phy-mode = "sgmii"; means. Should
> this include the overclocked 2.5G speed, or should we add a 2500sgmii
> link mode?

No.

>
>      Andrew

Greats,

René

[0]:  
https://www.realtek.com/en/products/communications-network-ics/item/rtl8367s-cg
[1]:  
https://drive.google.com/file/d/1cW8KQmmVpwDGmBd48KNQes9CRn7FEgBb/view?usp=sharing

^ permalink raw reply

* Re: Controllers with several interface options - one or more drivers?
From: Daniel Vetter @ 2019-07-26 15:06 UTC (permalink / raw)
  To: Sam Ravnborg, Laurent Pinchart, Andrzej Hajda
  Cc: devicetree, Dave Airlie, Josef Lusticky, dri-devel,
	Thierry Reding
In-Reply-To: <20190726145513.GK15868@phenom.ffwll.local>

Also probably should add a few more (drm_bridge) people, I think
that's also somewhat relevant here.
-Daniel

On Fri, Jul 26, 2019 at 4:55 PM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Fri, Jul 26, 2019 at 02:25:10PM +0200, Sam Ravnborg wrote:
> > Hi Josef, Daniel et al.
> >
> > The driver that triggered this reply is a driver that adds parallel
> > support to ili9341 in a dedicated panel driver.
> > The issue here is that we already have a tiny driver that supports the
> > ili9341 controller - but with a slightly different configuration.
> >
> > The ili9341 supports several interfaces - from the datasheet:
> >     "ILI9341 supports parallel 8-/9-/16-/18-bit data bus
> >      MCU interface, 6-/16-/18-bit data bus RGB interface and
> >      3-/4-line serial peripheral interface (SPI)"
> >
> > Noralf - in another mail explained:
> > "
> > The MIPI Alliance has lots of standards some wrt. display controller
> > interfaces:
> > - MIPI DBI - Display Bus Interface (used for commands and optionally pixels)
> > - MIPI DPI - Display Pixel Interface (also called RGB interface or
> > DOTCLK interface)
> > - MIPI DSI - Display Serial Interface (commands and pixels)
> >
> > The ili9341 supports both MIPI DBI and DPI.
> > "
> >
> > MIPI DPI - is a good fit for a drm_panel driver.
> > MIPI DBI - requires a full display controller driver.
> >
> > There are many other examples of driver SoC that in the same way
> > can be seen only as a panel or as a full display controller driver.
> >
> > The open question here is if we should try to support both cases in the
> > same driver / file. Or shall we implment two different drivers.
> > One for the panel use-case. And one for the display controller usecase?
> >
> > Not sure - so asking for feedback.
>
> I'm not sure. Currently we do have DSI and dumb RGB panels all in
> drm/panel. I don't think we have DBI panels in there yet, but then
> drm/tiny is the only one supporting these.
>
> I guess we could look into move some of the DBI panel drivers into panel
> drivers, but that needs a bit more glue all around. I'm honestly not sure
> how the current DSI drivers in drm_panel work exactly, especially for
> command mode.
>
> Or maybe we need a new interface for command mode.
>
> Wrt sharing code between drivers for the same chip, but different
> interfaces: I wouldn't worry too much about that. Maybe try to have a
> shared header file at least for registers. Long term we could end up with
> one driver module which exposes different flavours of the same chip, so
> multiple drm_panel drivers, or maybe we'll get something more specific for
> dsi/dbi.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: Controllers with several interface options - one or more drivers?
From: Daniel Vetter @ 2019-07-26 14:55 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, airlied, Josef Lusticky, dri-devel, thierry.reding
In-Reply-To: <20190726122510.GA14341@ravnborg.org>

On Fri, Jul 26, 2019 at 02:25:10PM +0200, Sam Ravnborg wrote:
> Hi Josef, Daniel et al.
> 
> The driver that triggered this reply is a driver that adds parallel
> support to ili9341 in a dedicated panel driver.
> The issue here is that we already have a tiny driver that supports the
> ili9341 controller - but with a slightly different configuration.
> 
> The ili9341 supports several interfaces - from the datasheet:
>     "ILI9341 supports parallel 8-/9-/16-/18-bit data bus
>      MCU interface, 6-/16-/18-bit data bus RGB interface and
>      3-/4-line serial peripheral interface (SPI)"
> 
> Noralf - in another mail explained:
> "
> The MIPI Alliance has lots of standards some wrt. display controller
> interfaces:
> - MIPI DBI - Display Bus Interface (used for commands and optionally pixels)
> - MIPI DPI - Display Pixel Interface (also called RGB interface or
> DOTCLK interface)
> - MIPI DSI - Display Serial Interface (commands and pixels)
> 
> The ili9341 supports both MIPI DBI and DPI.
> "
> 
> MIPI DPI - is a good fit for a drm_panel driver.
> MIPI DBI - requires a full display controller driver.
> 
> There are many other examples of driver SoC that in the same way
> can be seen only as a panel or as a full display controller driver.
> 
> The open question here is if we should try to support both cases in the
> same driver / file. Or shall we implment two different drivers.
> One for the panel use-case. And one for the display controller usecase?
> 
> Not sure - so asking for feedback.

I'm not sure. Currently we do have DSI and dumb RGB panels all in
drm/panel. I don't think we have DBI panels in there yet, but then
drm/tiny is the only one supporting these.

I guess we could look into move some of the DBI panel drivers into panel
drivers, but that needs a bit more glue all around. I'm honestly not sure
how the current DSI drivers in drm_panel work exactly, especially for
command mode.

Or maybe we need a new interface for command mode.

Wrt sharing code between drivers for the same chip, but different
interfaces: I wouldn't worry too much about that. Maybe try to have a
shared header file at least for registers. Long term we could end up with
one driver module which exposes different flavours of the same chip, so
multiple drm_panel drivers, or maybe we'll get something more specific for
dsi/dbi.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v9 0/8] EDAC drivers for Armada XP L2 and DDR
From: James Morse @ 2019-07-26 14:53 UTC (permalink / raw)
  To: Chris Packham
  Cc: mark.rutland, devicetree, jlu, linux-edac, linux, linux-kernel,
	robh+dt, bp, mchehab, linux-arm-kernel, patches
In-Reply-To: <20190712034904.5747-1-chris.packham@alliedtelesis.co.nz>

Hi Chris,

On 12/07/2019 04:48, Chris Packham wrote:
> I still seem to be struggling to get this on anyone's radar.

Whose radar does it need to cross?


> The Reviews/Acks have been given so this should be good to go in via the ARM
> tree as planned.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/525561.html

For your v8 I took this to mean this series was done!

If nothing has changed with Boris and Russell's decision (it was two years ago....),
details of the patch system are here:

https://lore.kernel.org/linux-arm-kernel/20190624142346.pxljv3m4npatdiyk@shell.armlinux.org.uk/


Thanks,

James

^ permalink raw reply

* Re: [PATCH v9 8/8] EDAC: armada_xp: Add support for more SoCs
From: James Morse @ 2019-07-26 14:51 UTC (permalink / raw)
  To: Chris Packham
  Cc: bp, robh+dt, mark.rutland, linux, patches, mchehab, jlu,
	devicetree, linux-arm-kernel, linux-edac, linux-kernel
In-Reply-To: <20190712034904.5747-9-chris.packham@alliedtelesis.co.nz>

Hi Chris,

On 12/07/2019 04:49, Chris Packham wrote:
> The Armada 38x and other integrated SoCs use a reduced pin count so the
> width of the SDRAM interface is smaller than the Armada XP SoCs. This
> means that the definition of "full" and "half" width is reduced from
> 64/32 to 32/16.

> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
> index 3759a4fbbdee..7f227bdcbc84 100644
> --- a/drivers/edac/armada_xp_edac.c
> +++ b/drivers/edac/armada_xp_edac.c
> @@ -332,6 +332,11 @@ static int axp_mc_probe(struct platform_device *pdev)
>  
>  	axp_mc_read_config(mci);
>  
> +	/* These SoCs have a reduced width bus */
> +	if (of_machine_is_compatible("marvell,armada380") ||
> +	    of_machine_is_compatible("marvell,armadaxp-98dx3236"))
> +		drvdata->width /= 2;

So the hardware's SDRAM_CONFIG_BUS_WIDTH value is wrong? Yuck.

Is it too late for the DTs on these two systems to provide a DT version of the 'bus_width'
to override the hardware's mis-advertised value?

This way you don't need to grow this list.

Acked-by: James Morse <james.morse@arm.com>


Thanks,

James

^ permalink raw reply

* Re: [PATCH v7 0/7] Solve postboot supplier cleanup and optimize probe ordering
From: Greg Kroah-Hartman @ 2019-07-26 14:32 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Saravana Kannan, Rob Herring, Mark Rutland, Rafael J. Wysocki,
	devicetree, linux-kernel, David Collins, kernel-team
In-Reply-To: <99ca3252-55af-8eea-7653-8347b0a1ab03@gmail.com>

On Thu, Jul 25, 2019 at 02:04:23PM -0700, Frank Rowand wrote:
> On 7/25/19 6:42 AM, Greg Kroah-Hartman wrote:
> > On Tue, Jul 23, 2019 at 05:10:53PM -0700, Saravana Kannan wrote:
> >> Add device-links to track functional dependencies between devices
> >> after they are created (but before they are probed) by looking at
> >> their common DT bindings like clocks, interconnects, etc.
> >>
> >> Having functional dependencies automatically added before the devices
> >> are probed, provides the following benefits:
> >>
> >> - Optimizes device probe order and avoids the useless work of
> >>   attempting probes of devices that will not probe successfully
> >>   (because their suppliers aren't present or haven't probed yet).
> >>
> >>   For example, in a commonly available mobile SoC, registering just
> >>   one consumer device's driver at an initcall level earlier than the
> >>   supplier device's driver causes 11 failed probe attempts before the
> >>   consumer device probes successfully. This was with a kernel with all
> >>   the drivers statically compiled in. This problem gets a lot worse if
> >>   all the drivers are loaded as modules without direct symbol
> >>   dependencies.
> >>
> >> - Supplier devices like clock providers, interconnect providers, etc
> >>   need to keep the resources they provide active and at a particular
> >>   state(s) during boot up even if their current set of consumers don't
> >>   request the resource to be active. This is because the rest of the
> >>   consumers might not have probed yet and turning off the resource
> >>   before all the consumers have probed could lead to a hang or
> >>   undesired user experience.
> >>
> >>   Some frameworks (Eg: regulator) handle this today by turning off
> >>   "unused" resources at late_initcall_sync and hoping all the devices
> >>   have probed by then. This is not a valid assumption for systems with
> >>   loadable modules. Other frameworks (Eg: clock) just don't handle
> >>   this due to the lack of a clear signal for when they can turn off
> >>   resources. This leads to downstream hacks to handle cases like this
> >>   that can easily be solved in the upstream kernel.
> >>
> >>   By linking devices before they are probed, we give suppliers a clear
> >>   count of the number of dependent consumers. Once all of the
> >>   consumers are active, the suppliers can turn off the unused
> >>   resources without making assumptions about the number of consumers.
> >>
> >> By default we just add device-links to track "driver presence" (probe
> >> succeeded) of the supplier device. If any other functionality provided
> >> by device-links are needed, it is left to the consumer/supplier
> >> devices to change the link when they probe.
> >>
> >> v1 -> v2:
> >> - Drop patch to speed up of_find_device_by_node()
> >> - Drop depends-on property and use existing bindings
> >>
> >> v2 -> v3:
> >> - Refactor the code to have driver core initiate the linking of devs
> >> - Have driver core link consumers to supplier before it's probed
> >> - Add support for drivers to edit the device links before probing
> >>
> >> v3 -> v4:
> >> - Tested edit_links() on system with cyclic dependency. Works.
> >> - Added some checks to make sure device link isn't attempted from
> >>   parent device node to child device node.
> >> - Added way to pause/resume sync_state callbacks across
> >>   of_platform_populate().
> >> - Recursively parse DT node to create device links from parent to
> >>   suppliers of parent and all child nodes.
> >>
> >> v4 -> v5:
> >> - Fixed copy-pasta bugs with linked list handling
> >> - Walk up the phandle reference till I find an actual device (needed
> >>   for regulators to work)
> >> - Added support for linking devices from regulator DT bindings
> >> - Tested the whole series again to make sure cyclic dependencies are
> >>   broken with edit_links() and regulator links are created properly.
> >>
> >> v5 -> v6:
> >> - Split, squashed and reordered some of the patches.
> >> - Refactored the device linking code to follow the same code pattern for
> >>   any property.
> >>
> >> v6 -> v7:
> >> - No functional changes.
> >> - Renamed i to index
> >> - Added comment to clarify not having to check property name for every
> >>   index
> >> - Added "matched" variable to clarify code. No functional change.
> >> - Added comments to include/linux/device.h for add_links()
> >>
> >> I've also not updated this patch series to handle the new patch [1] from
> >> Rafael. Will do that once this patch series is close to being Acked.
> >>
> >> [1] - https://lore.kernel.org/lkml/3121545.4lOhFoIcdQ@kreacher/
> > 
> > 
> > This looks sane to me.  Anyone have any objections for me queueing this
> > up for my tree to get into linux-next now?
> 
> I would like for the series to get into linux-next sooner than later,
> and spend some time there.  

Ok, care to give me an ack for it?  :)

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2 0/2] Add Sharp panel option for Lenovo Miix 630
From: Jeffrey Hugo @ 2019-07-26 14:31 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Rob Herring, Mark Rutland, thierry.reding, Dave Airlie,
	Daniel Vetter, Bjorn Andersson, open list:DRM PANEL DRIVERS, DTML,
	MSM, lkml
In-Reply-To: <20190726123625.GA17037@ravnborg.org>

On Fri, Jul 26, 2019 at 6:36 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Jeffrey.
>
> On Mon, Jul 08, 2019 at 09:56:47AM -0700, Jeffrey Hugo wrote:
> > The Lenovo Miix 630 laptop can be found with one of two panels - a BOE
> > or Sharp option.  This likely provides options during manufacturing.
> >
> > These panels connect via eDP, however they sit behind a DSI to eDP
> > bridge on the laptop, so they can easily be handled by the existing
> > simple panel code.
> >
> > This series adds support for the Sharp option.
> >
> > v2:
> > -removed no-hpd from dt example
> > -added .bus_format and .bus_flags fields based on reviews
> > -added .flags after Bjorn pointed me to something I missed
> > -added Sam's reviewed-by tags
> >
> > Jeffrey Hugo (2):
> >   dt-bindings: panel: Add Sharp LD-D5116Z01B
> >   drm/panel: simple: Add support for Sharp LD-D5116Z01B panel
>
> Thanks.
> Both patches applied and pushed to drm-misc-next.

Excellent.  Thanks

> Are you up to a little janitorial work?
> Today the preferred format for bindings files are the new yaml format.
> Could you update 'your' file, and maybe the other sharp files too?

I confess I haven't yet familiarized myself with the yaml format yet,
but I'll take a look and do an update once I understand the
requirements.

^ permalink raw reply

* Re: [PATCH 1/7] docs: fix broken doc references due to renames
From: Daniel Vetter @ 2019-07-26 14:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Andrea Parri, Ajay Gupta, Linux Doc Mailing List,
	Peter Zijlstra, Akira Yokosawa, Lai Jiangshan, Jerry Hoemann,
	dri-devel, David Howells, Mauro Carvalho Chehab, Linux I2C,
	Joel Fernandes, Paul E. McKenney, Will Deacon,
	open list:GENERIC INCLUDE/ASM HEADER FILES, SCSI, Jonathan Corbet,
	Wolfram Sang, esc.storagedev, Maxime Ripard,
	Ingo Molnar <mingo@
In-Reply-To: <CAL_JsqK_rfHehrKW_NS89BOV0=dYoao0H=zOzG=D-724vKduKw@mail.gmail.com>

On Fri, Jul 26, 2019 at 07:41:35AM -0600, Rob Herring wrote:
> On Fri, Jul 26, 2019 at 5:47 AM Mauro Carvalho Chehab
> <mchehab+samsung@kernel.org> wrote:
> >
> > Some files got renamed but probably due to some merge conflicts,
> > a few references still point to the old locations.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > Acked-by: Wolfram Sang <wsa@the-dreams.de> # I2C part
> > Reviewed-by: Jerry Hoemann <jerry.hoemann@hpe.com> # hpwdt.rst
> > ---
> >  Documentation/RCU/rculist_nulls.txt                   |  2 +-
> >  Documentation/devicetree/bindings/arm/idle-states.txt |  2 +-
> >  Documentation/locking/spinlocks.rst                   |  4 ++--
> >  Documentation/memory-barriers.txt                     |  2 +-
> >  Documentation/translations/ko_KR/memory-barriers.txt  |  2 +-
> >  Documentation/watchdog/hpwdt.rst                      |  2 +-
> >  MAINTAINERS                                           | 10 +++++-----
> >  drivers/gpu/drm/drm_modes.c                           |  2 +-

for the drm part:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> >  drivers/i2c/busses/i2c-nvidia-gpu.c                   |  2 +-
> >  drivers/scsi/hpsa.c                                   |  4 ++--
> >  10 files changed, 16 insertions(+), 16 deletions(-)
> 
> Acked-by: Rob Herring <robh@kernel.org>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 2/3] usb: exynos: add support for getting PHYs from the standard dt array
From: Alan Stern @ 2019-07-26 14:17 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-usb, linux-samsung-soc, linux-kernel, devicetree,
	Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz, Markus Reichl,
	Måns Rullgård, Krzysztof Kozlowski, Peter Chen,
	Rob Herring
In-Reply-To: <20190726081453.9456-3-m.szyprowski@samsung.com>

On Fri, 26 Jul 2019, Marek Szyprowski wrote:

> Add the code for getting generic PHYs from standard device tree array
> from the main controller device node. This is a first step in resolving
> the conflict between Exynos EHCI/OHCI sub-nodes and generic USB device
> bindings. Later the sub-nodes currently used for assigning PHYs to root
> ports of the controller will be removed making a place for the generic
> USB device bindings nodes.
> 
> Suggested-by: Måns Rullgård <mans@mansr.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Acked-by: Alan Stern <stern@rowland.harvard.edu>

>  drivers/usb/host/ehci-exynos.c | 23 +++++++++++++++++++----
>  drivers/usb/host/ohci-exynos.c | 23 +++++++++++++++++++----
>  2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index 3a29a1a8519c..01debfd03d4a 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -41,6 +41,7 @@ struct exynos_ehci_hcd {
>  	struct clk *clk;
>  	struct device_node *of_node;
>  	struct phy *phy[PHY_NUMBER];
> +	bool legacy_phy;
>  };
>  
>  #define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
> @@ -50,10 +51,22 @@ static int exynos_ehci_get_phy(struct device *dev,
>  {
>  	struct device_node *child;
>  	struct phy *phy;
> -	int phy_number;
> +	int phy_number, num_phys;
>  	int ret;
>  
>  	/* Get PHYs for the controller */
> +	num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> +					      "#phy-cells");
> +	for (phy_number = 0; phy_number < num_phys; phy_number++) {
> +		phy = devm_of_phy_get_by_index(dev, dev->of_node, phy_number);
> +		if (IS_ERR(phy))
> +			return PTR_ERR(phy);
> +		exynos_ehci->phy[phy_number] = phy;
> +	}
> +	if (num_phys > 0)
> +		return 0;
> +
> +	/* Get PHYs using legacy bindings */
>  	for_each_available_child_of_node(dev->of_node, child) {
>  		ret = of_property_read_u32(child, "reg", &phy_number);
>  		if (ret) {
> @@ -84,6 +97,7 @@ static int exynos_ehci_get_phy(struct device *dev,
>  		}
>  	}
>  
> +	exynos_ehci->legacy_phy = true;
>  	return 0;
>  }
>  
> @@ -205,11 +219,12 @@ static int exynos_ehci_probe(struct platform_device *pdev)
>  	ehci->caps = hcd->regs;
>  
>  	/*
> -	 * Workaround: reset of_node pointer to avoid conflict between Exynos
> -	 * EHCI port subnodes and generic USB device bindings
> +	 * Workaround: reset of_node pointer to avoid conflict between legacy
> +	 * Exynos EHCI port subnodes and generic USB device bindings
>  	 */
>  	exynos_ehci->of_node = pdev->dev.of_node;
> -	pdev->dev.of_node = NULL;
> +	if (exynos_ehci->legacy_phy)
> +		pdev->dev.of_node = NULL;
>  
>  	/* DMA burst Enable */
>  	writel(EHCI_INSNREG00_ENABLE_DMA_BURST, EHCI_INSNREG00(hcd->regs));
> diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
> index 905c6317e0c3..d5ce98e205c7 100644
> --- a/drivers/usb/host/ohci-exynos.c
> +++ b/drivers/usb/host/ohci-exynos.c
> @@ -32,6 +32,7 @@ struct exynos_ohci_hcd {
>  	struct clk *clk;
>  	struct device_node *of_node;
>  	struct phy *phy[PHY_NUMBER];
> +	bool legacy_phy;
>  };
>  
>  static int exynos_ohci_get_phy(struct device *dev,
> @@ -39,10 +40,22 @@ static int exynos_ohci_get_phy(struct device *dev,
>  {
>  	struct device_node *child;
>  	struct phy *phy;
> -	int phy_number;
> +	int phy_number, num_phys;
>  	int ret;
>  
>  	/* Get PHYs for the controller */
> +	num_phys = of_count_phandle_with_args(dev->of_node, "phys",
> +					      "#phy-cells");
> +	for (phy_number = 0; phy_number < num_phys; phy_number++) {
> +		phy = devm_of_phy_get_by_index(dev, dev->of_node, phy_number);
> +		if (IS_ERR(phy))
> +			return PTR_ERR(phy);
> +		exynos_ohci->phy[phy_number] = phy;
> +	}
> +	if (num_phys > 0)
> +		return 0;
> +
> +	/* Get PHYs using legacy bindings */
>  	for_each_available_child_of_node(dev->of_node, child) {
>  		ret = of_property_read_u32(child, "reg", &phy_number);
>  		if (ret) {
> @@ -73,6 +86,7 @@ static int exynos_ohci_get_phy(struct device *dev,
>  		}
>  	}
>  
> +	exynos_ohci->legacy_phy = true;
>  	return 0;
>  }
>  
> @@ -172,11 +186,12 @@ static int exynos_ohci_probe(struct platform_device *pdev)
>  	}
>  
>  	/*
> -	 * Workaround: reset of_node pointer to avoid conflict between Exynos
> -	 * OHCI port subnodes and generic USB device bindings
> +	 * Workaround: reset of_node pointer to avoid conflict between legacy
> +	 * Exynos OHCI port subnodes and generic USB device bindings
>  	 */
>  	exynos_ohci->of_node = pdev->dev.of_node;
> -	pdev->dev.of_node = NULL;
> +	if (exynos_ohci->legacy_phy)
> +		pdev->dev.of_node = NULL;
>  
>  	err = usb_add_hcd(hcd, irq, IRQF_SHARED);
>  	if (err) {
> 

^ permalink raw reply

* Re: [RFC, v3 9/9] media: platform: Add Mediatek ISP P1 shared memory device
From: Tomasz Figa @ 2019-07-26 14:04 UTC (permalink / raw)
  To: Jungo Lin, Robin Murphy
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	Sean Cheng (鄭昇弘),
	Frederic Chen (陳俊元),
	Rynn Wu (吳育恩), srv_heupstream, Rob Herring,
	Ryan Yu (余孟修),
	Frankie Chiu (邱文凱),
	list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS, Matthias Brugger,
	Sj Huang, moderated list:ARM/Mediatek SoC support,
	Laurent Pinchart, Hans Verkuil, ddavenport-F7+t8E8rja9g9hUCZPvPmw,
	Mauro Carvalho Chehab, list-Y9sIeH5OGRo
In-Reply-To: <1564142386.1212.621.camel@mtksdccf07>

On Fri, Jul 26, 2019 at 8:59 PM Jungo Lin <jungo.lin-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>
> Hi Robin:
>
> On Fri, 2019-07-26 at 12:04 +0100, Robin Murphy wrote:
> > On 26/07/2019 08:42, Tomasz Figa wrote:
> > > On Fri, Jul 26, 2019 at 4:41 PM Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> > >>
> > >> On Fri, Jul 26, 2019 at 02:15:14PM +0900, Tomasz Figa wrote:
> > >>> Could you try dma_get_sgtable() with the SCP struct device and then
> > >>> dma_map_sg() with the P1 struct device?
> > >>
> > >> Please don't do that.  dma_get_sgtable is a pretty broken API (see
> > >> the common near the arm implementation) and we should not add more
> > >> users of it.  If you want a piece of memory that can be mapped to
> > >> multiple devices allocate it using alloc_pages and then just map
> > >> it to each device.
> > >
> > > Thanks for taking a look at this thread.
> > >
> > > Unfortunately that wouldn't work. We have a specific reserved memory
> > > pool that is the only memory area accessible to one of the devices.
> > > Any idea how to handle this?
> >
> > If it's reserved in the sense of being outside struct-page-backed
> > "kernel memory", then provided you have a consistent CPU physical
> > address it might be reasonable for other devices to access it via
> > dma_map_resource().
> >
> > Robin.
>
> Thank you for your suggestion.
>
> After revising to use dma_map_resource(), it is worked. Below is the
> current implementation. Pleas kindly help us to check if there is any
> misunderstanding.
>
> #define MTK_ISP_COMPOSER_MEM_SIZE               0x200000
>
>         /*
>          * Allocate coherent reserved memory for SCP firmware usage.
>          * The size of SCP composer's memory is fixed to 0x200000
>          * for the requirement of firmware.
>          */
>         ptr = dma_alloc_coherent(p1_dev->cam_dev.smem_dev,
>                                  MTK_ISP_COMPOSER_MEM_SIZE, &addr, GFP_KERNEL);
>         if (!ptr) {
>                 dev_err(dev, "failed to allocate compose memory\n");
>                 return -ENOMEM;
>         }
>         p1_dev->composer_scp_addr = addr;
>         p1_dev->composer_virt_addr = ptr;
>         dev_dbg(dev, "scp addr:%pad va:%pK\n", &addr, ptr);
>
>         /*
>          * This reserved memory is also be used by ISP P1 HW.
>          * Need to get iova address for ISP P1 DMA.
>          */
>         addr = dma_map_resource(dev, addr, MTK_ISP_COMPOSER_MEM_SIZE,
>                                 DMA_BIDIRECTIONAL, DMA_ATTR_SKIP_CPU_SYNC);

This is still incorrect, because addr is a DMA address, but the second
argument to dma_map_resource() is a physical address.

>         if (dma_mapping_error(dev, addr)) {
>                 dev_err(dev, "Failed to map scp iova\n");
>                 ret = -ENOMEM;
>                 goto fail_free_mem;
>         }
>         p1_dev->composer_iova = addr;
>         dev_info(dev, "scp iova addr:%pad\n", &addr);
>
> Moreover, appropriate Tomasz & Christoph's help on this issue.

Robin, the memory is specified using the reserved-memory DT binding
and managed by the coherent DMA pool framework. We can allocate from
it using dma_alloc_coherent(), which gives us a DMA address, not CPU
physial address (although in practice on this platform they are equal
numerically).

Best regards,
Tomasz

^ permalink raw reply

* [PATCH 3/5] dt-bindings: arm: Extend SCMI to support new reset protocol
From: Sudeep Holla @ 2019-07-26 13:59 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Sudeep Holla, Peng Fan, linux-kernel, Bo Zhang, Jim Quinlan,
	Volodymyr Babchuk, Gaku Inami, aidapala, pajay, Etienne Carriere,
	Souvik Chakravarty, wesleys, Felix Burton, Saeed Nowshadi,
	Philipp Zabel, Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20190726135954.11078-1-sudeep.holla@arm.com>

SCMIv2.0 adds a new Reset Management Protocol to manage various reset
states a given device or domain can enter. Extend the existing SCMI
bindings to add reset protocol support by re-using the reset bindings
for bothe reset providers and consumers.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 .../devicetree/bindings/arm/arm,scmi.txt        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt
index 317a2fc3667a..083dbf96ee00 100644
--- a/Documentation/devicetree/bindings/arm/arm,scmi.txt
+++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt
@@ -73,6 +73,16 @@ SCMI provides an API to access the various sensors on the SoC.
 			 as used by the firmware. Refer to  platform details
 			 for your implementation for the IDs to use.
 
+Reset signal bindings for the reset domains based on SCMI Message Protocol
+------------------------------------------------------------
+
+This binding for the SCMI reset domain providers uses the generic reset
+signal binding[5].
+
+Required properties:
+ - #reset-cells : Should be 1. Contains the reset domain ID value used
+		  by SCMI commands.
+
 SRAM and Shared Memory for SCMI
 -------------------------------
 
@@ -93,6 +103,7 @@ Each sub-node represents the reserved area for SCMI.
 [2] Documentation/devicetree/bindings/power/power_domain.txt
 [3] Documentation/devicetree/bindings/thermal/thermal.txt
 [4] Documentation/devicetree/bindings/sram/sram.txt
+[5] Documentation/devicetree/bindings/reset/reset.txt
 
 Example:
 
@@ -152,6 +163,11 @@ firmware {
 			reg = <0x15>;
 			#thermal-sensor-cells = <1>;
 		};
+
+		scmi_reset: protocol@16 {
+			reg = <0x16>;
+			#reset-cells = <1>;
+		};
 	};
 };
 
@@ -166,6 +182,7 @@ hdlcd@7ff60000 {
 	reg = <0 0x7ff60000 0 0x1000>;
 	clocks = <&scmi_clk 4>;
 	power-domains = <&scmi_devpd 1>;
+	resets = <&scmi_reset 10>;
 };
 
 thermal-zones {
-- 
2.17.1

^ permalink raw reply related


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