Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] drm/bridge: Add RGB to VGA bridge support
From: Laurent Pinchart @ 2016-09-18 10:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2115773.yQojqqIjGI@avalon>

On Sunday 18 Sep 2016 13:01:16 Laurent Pinchart wrote:
> On Thursday 08 Sep 2016 14:17:48 Maxime Ripard wrote:
> > Some boards have an entirely passive RGB to VGA bridge, based on either
> > DACs or resistor ladders.
> > 
> > Those might or might not have an i2c bus routed to the VGA connector in
> > order to access the screen EDIDs.
> > 
> > Add a bridge that doesn't do anything but expose the modes available on
> > the screen, either based on the EDIDs if available, or based on the XGA
> > standards.
> > 
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> > 
> >  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  52 +++++
> >  drivers/gpu/drm/bridge/Kconfig                     |   6 +
> >  drivers/gpu/drm/bridge/Makefile                    |   1 +
> >  drivers/gpu/drm/bridge/rgb-to-vga.c                | 232 ++++++++++++++++
> >  4 files changed, 291 insertions(+)
> >  create mode 100644
> > 
> > Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > new file mode 100644
> > index 000000000000..83a053fb51a0
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> > @@ -0,0 +1,52 @@
> > +Passive RGB to VGA bridge
> > +-------------------------
> > +
> > +This binding is aimed for entirely passive RGB to VGA bridges that do not
> > +require any configuration.
> 
> Couldn't it also support active RGB to VGA bridges that don't require any
> configuration ? It would seem a bit pointless to define a separate DT
> binding for them.

I'm thinking in particular about the ADV7123 
(http://www.analog.com/media/en/technical-documentation/data-sheets/ADV7123.pdf) that already has a DT binding. Would it be feasible to 
combine the two, and support both devices with a single driver ?

> > +Required properties:
> > +
> > +- compatible: Must be "rgb-to-vga-bridge"
> > +
> > +Required nodes:
> > +
> > +This device has two video ports. Their connections are modeled using the
> > OF +graph bindings specified in
> > Documentation/devicetree/bindings/graph.txt. +
> > +- Video port 0 for RGB input
> > +- Video port 1 for VGA output
> > +
> > +
> > +Example
> > +-------
> > +
> > +bridge {
> > +	compatible = "rgb-to-vga-bridge";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		port at 0 {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			reg = <0>;
> > +
> > +			vga_bridge_in: endpoint {
> > +				remote-endpoint = <&tcon0_out_vga>;
> > +			};
> > +		};
> > +
> > +		port at 1 {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +			reg = <1>;
> > +
> > +			vga_bridge_out: endpoint {
> > +				remote-endpoint = <&vga_con_in>;
> > +			};
> > +		};
> > +	};
> > +};

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v3 1/4] drm/bridge: Add RGB to VGA bridge support
From: Laurent Pinchart @ 2016-09-18 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160908121751.16911-2-maxime.ripard@free-electrons.com>

Hi Maxime,

Thank you for the patch.

On Thursday 08 Sep 2016 14:17:48 Maxime Ripard wrote:
> Some boards have an entirely passive RGB to VGA bridge, based on either
> DACs or resistor ladders.
> 
> Those might or might not have an i2c bus routed to the VGA connector in
> order to access the screen EDIDs.
> 
> Add a bridge that doesn't do anything but expose the modes available on the
> screen, either based on the EDIDs if available, or based on the XGA
> standards.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  52 +++++
>  drivers/gpu/drm/bridge/Kconfig                     |   6 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/rgb-to-vga.c                | 232 ++++++++++++++++++
>  4 files changed, 291 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> new file mode 100644
> index 000000000000..83a053fb51a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
> @@ -0,0 +1,52 @@
> +Passive RGB to VGA bridge
> +-------------------------
> +
> +This binding is aimed for entirely passive RGB to VGA bridges that do not
> +require any configuration.

Couldn't it also support active RGB to VGA bridges that don't require any 
configuration ? It would seem a bit pointless to define a separate DT binding 
for them.

> +Required properties:
> +
> +- compatible: Must be "rgb-to-vga-bridge"
> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modeled using the OF
> +graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for RGB input
> +- Video port 1 for VGA output
> +
> +
> +Example
> +-------
> +
> +bridge {
> +	compatible = "rgb-to-vga-bridge";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port at 0 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0>;
> +
> +			vga_bridge_in: endpoint {
> +				remote-endpoint = <&tcon0_out_vga>;
> +			};
> +		};
> +
> +		port at 1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +
> +			vga_bridge_out: endpoint {
> +				remote-endpoint = <&vga_con_in>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index b590e678052d..42b95adf5091 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -17,6 +17,12 @@ config DRM_ANALOGIX_ANX78XX
>  	  the HDMI output of an application processor to MyDP
>  	  or DisplayPort.
> 
> +config DRM_RGB_TO_VGA
> +	tristate "Dumb RGB to VGA Bridge support"
> +	select DRM_KMS_HELPER
> +	help
> +	  Support for passive RGB to VGA bridges
> +
>  config DRM_DW_HDMI
>  	tristate
>  	select DRM_KMS_HELPER
> diff --git a/drivers/gpu/drm/bridge/Makefile
> b/drivers/gpu/drm/bridge/Makefile index efdb07e878f5..3bb8cbe09fe9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,6 +1,7 @@
>  ccflags-y := -Iinclude/drm
> 
>  obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> +obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
>  obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c
> b/drivers/gpu/drm/bridge/rgb-to-vga.c new file mode 100644
> index 000000000000..84b1b10198a4
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
> @@ -0,0 +1,232 @@
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct dumb_vga {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +};
> +
> +static inline struct dumb_vga *
> +drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct dumb_vga, bridge);
> +}
> +
> +static inline struct dumb_vga *
> +drm_connector_to_dumb_vga(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct dumb_vga, connector);
> +}
> +
> +static int dumb_vga_get_modes(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (IS_ERR(vga->ddc))
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, vga->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID readout failed, falling back to standard 
modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	return drm_add_edid_modes(connector, edid);
> +
> +fallback:
> +	/*
> +	 * In case we cannot retrieve the EDIDs (broken or missing i2c
> +	 * bus), fallback on the XGA standards
> +	 */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anyone can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static struct drm_encoder *
> +dumb_vga_best_encoder(struct drm_connector *connector)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	return vga->bridge.encoder;
> +}
> +
> +static struct drm_connector_helper_funcs dumb_vga_con_helper_funcs = {
> +	.get_modes	= dumb_vga_get_modes,
> +	.best_encoder	= dumb_vga_best_encoder,
> +};
> +
> +static enum drm_connector_status
> +dumb_vga_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
> +
> +	/*
> +	 * Even if we have an I2C bus, we can't assume that the cable
> +	 * is disconnected if drm_probe_ddc. Some cables don't wire
> +	 * the DDC pins, or the I2C bus might be disfunctional.
> +	 */
> +	if (!IS_ERR(vga->ddc) && drm_probe_ddc(vga->ddc))
> +		return connector_status_connected;
> +
> +	return connector_status_unknown;
> +}
> +
> +static void
> +dumb_vga_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_cleanup(connector);
> +}
> +
> +static struct drm_connector_funcs dumb_vga_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= dumb_vga_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= dumb_vga_connector_destroy,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int dumb_vga_attach(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&vga->connector,
> +				 &dumb_vga_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &vga->connector,
> +				 &dumb_vga_con_funcs, DRM_MODE_CONNECTOR_VGA);

Bridges should really not create connectors. Fixing this problem is a bit out 
of scope for your patch, but I'm concerned that the growing number of bridge 
drivers will make it difficult to fix it later.

> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&vga->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static void dumb_vga_nop(struct drm_bridge *bridge) {};
> +
> +static struct drm_bridge_funcs dumb_vga_bridge_funcs = {
> +	.attach		= dumb_vga_attach,
> +	.enable		= dumb_vga_nop,
> +	.disable	= dumb_vga_nop,
> +	.pre_enable	= dumb_vga_nop,
> +	.post_disable	= dumb_vga_nop,

Those four operations are optional, you don't need dumb_vga_nop().

> +};
> +
> +static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> +{
> +	struct device_node *end_node, *phandle, *remote;
> +	struct i2c_adapter *ddc;
> +
> +	end_node = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!end_node) {
> +		dev_err(dev, "Missing connector endpoint\n");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
> +	remote = of_graph_get_remote_port_parent(end_node);
> +	of_node_put(end_node);
> +	if (!remote) {
> +		dev_err(dev, "Enable to parse remote node\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	phandle = of_parse_phandle(remote, "ddc-i2c-bus", 0);
> +	of_node_put(remote);
> +	if (!phandle)
> +		return ERR_PTR(-ENODEV);
> +
> +	ddc = of_get_i2c_adapter_by_node(phandle);
> +	of_node_put(phandle);
> +	if (!ddc)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return ddc;
> +}
> +
> +static int dumb_vga_probe(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga;
> +	int ret;
> +
> +	vga = devm_kzalloc(&pdev->dev, sizeof(*vga), GFP_KERNEL);
> +	if (!vga)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, vga);
> +
> +	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
> +	if (IS_ERR(vga->ddc)) {
> +		if (PTR_ERR(vga->ddc) == -ENODEV) {
> +			dev_info(&pdev->dev,
> +				 "No i2c bus specified... Disabling EDID 
readout\n");
> +		} else {
> +			dev_err(&pdev->dev, "Couldn't retrieve i2c bus\n");
> +			return PTR_ERR(vga->ddc);
> +		}
> +	}
> +
> +	vga->bridge.funcs = &dumb_vga_bridge_funcs;
> +	vga->bridge.of_node = pdev->dev.of_node;
> +
> +	ret = drm_bridge_add(&vga->bridge);
> +	if (ret && !IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return ret;
> +}
> +
> +static int dumb_vga_remove(struct platform_device *pdev)
> +{
> +	struct dumb_vga *vga = platform_get_drvdata(pdev);
> +
> +	drm_bridge_remove(&vga->bridge);
> +
> +	if (!IS_ERR(vga->ddc))
> +		i2c_put_adapter(vga->ddc);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id dumb_vga_match[] = {
> +	{ .compatible = "rgb-to-vga-bridge" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, dumb_vga_match);
> +
> +struct platform_driver dumb_vga_driver = {
> +	.probe	= dumb_vga_probe,
> +	.remove	= dumb_vga_remove,
> +	.driver		= {
> +		.name		= "rgb-to-vga-bridge",
> +		.of_match_table	= dumb_vga_match,
> +	},
> +};
> +module_platform_driver(dumb_vga_driver);
> +
> +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> +MODULE_DESCRIPTION("Dumb RGB to VGA bridge driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v6 2/3] ARM: dts: add TOPEET itop elite based board
From: ayaka @ 2016-09-18  9:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1e4335db-9004-bb7c-6dc1-e435e66b8126@osg.samsung.com>



On 09/17/2016 02:09 AM, Javier Martinez Canillas wrote:
> Hello Randy,
>
> On 09/16/2016 01:21 PM, Krzysztof Kozlowski wrote:
>
> [snip]
>
>>>>> +	
>>>>> +	beep {
>>>>> +		compatible = "pwm-beeper";
>>>>> +		pwms = <&pwm 0 4000000 PWM_POLARITY_INVERTED>;
>>>> I have serious doubts that you run this code... if it does not even
>>>> compile. Sorry, I cannot accept code that does not compile and was not
>>>> tested.
>>>>
>>>> Please, test your DTS on top of current Linux tree, which would be one of:
>>>> 1. Linus' master branch: v4.8-rc6,
>>>> 2. one of my branches (for-next, next/dt etc),
>>> When I sent those patches, I tested in "Add linux-next specific files for
>>> 20160907"
>> OK, good, but please tell me why kbuild reported that build error?
>>
> I agree with Krzysztof, it doesn't seem that you tested the exact version
I am sure I does.
> that was posted. You will need the following change to make the DTS build
> in latest linux-next:
I didn't post the patch is WIP, which would adding the LCD panel support 
to this board.
>
> diff --git a/arch/arm/boot/dts/exynos4412-itop-elite.dts b/arch/arm/boot/dts/exynos4412-itop-elite.dts
> index dd83689892ff..8559f891a740 100644
> --- a/arch/arm/boot/dts/exynos4412-itop-elite.dts
> +++ b/arch/arm/boot/dts/exynos4412-itop-elite.dts
> @@ -14,6 +14,7 @@
>   */
>   
>   /dts-v1/;
> +#include <dt-bindings/pwm/pwm.h>
>   #include <dt-bindings/sound/samsung-i2s.h>
>   #include "exynos4412-itop-scp-core.dtsi"
>
> So please squash that change and resend.
I would, but I meet a problem in memory with the latest next-20160916, 
it seems that it can't access the memory while it never happened before.
MMC read: dev # 1, block # 2048, count 14336 ...14336 blocks read: 
OK                                                     [58/426]
Boot with zImage

Starting kernel ...

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0xa00
[    0.000000] Linux version 4.8.0-rc6-next-20160916-00004-gae92137 
(ayaka at ritsuko) (gcc version 6.1.0 (Buildroot 2016.05-00006-g0
792d0d-dirty) ) #83 SMP PREEMPT Sun Sep 18 17:29:56 CST 2016
[    0.000000] CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), 
cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing 
instruction cache
[    0.000000] OF: fdt:Machine model: TOPEET iTop 4412 Elite board based 
on Exynos4412
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] OF: reserved mem: failed to allocate memory for node 
'region_mfc_right'
[    0.000000] OF: reserved mem: failed to allocate memory for node 
'region_mfc_left'
[    0.000000] cma: Failed to reserve 32 MiB
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] Kernel panic - not syncing: ERROR: Failed to allocate 
0x2000 bytes below 0x0.
[    0.000000]
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 
4.8.0-rc6-next-20160916-00004-gae92137 #83
[    0.000000] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[    0.000000] [<c010e17c>] (unwind_backtrace) from [<c010af78>] 
(show_stack+0x10/0x14)
[    0.000000] [<c010af78>] (show_stack) from [<c03080e4>] 
(dump_stack+0x70/0x8c)
[    0.000000] [<c03080e4>] (dump_stack) from [<c019e580>] 
(panic+0xc4/0x244)
[    0.000000] [<c019e580>] (panic) from [<c0810e3c>] 
(memblock_alloc_base+0x2c/0x38)
[    0.000000] [<c0810e3c>] (memblock_alloc_base) from [<c0806aec>] 
(early_alloc_aligned+0xc/0x2c)
[    0.000000] [<c0806aec>] (early_alloc_aligned) from [<c08074c8>] 
(paging_init+0x548/0xa70)
[    0.000000] [<c08074c8>] (paging_init) from [<c0803dc8>] 
(setup_arch+0x50c/0xa28)
[    0.000000] [<c0803dc8>] (setup_arch) from [<c080091c>] 
(start_kernel+0x50/0x384)
[    0.000000] [<c080091c>] (start_kernel) from [<4000807c>] (0x4000807c)
[    0.000000] ---[ end Kernel panic - not syncing: ERROR: Failed to 
allocate 0x2000 bytes below 0x0.
[    0.000000

>
> Best regards,

^ permalink raw reply

* [RFC PATCH 00/10] ARM: dts: exynos: Fix invalid GIC interrupt flags
From: Marc Zyngier @ 2016-09-18  9:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160917175553.GA9499@kozik-lap>

On Sat, 17 Sep 2016 19:55:53 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On Sat, Sep 17, 2016 at 11:05:39PM +0530, Alim Akhtar wrote:
> > Hi Krzysztof,
> > 
> > On Sat, Sep 17, 2016 at 1:12 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> > > Hi,
> > >
> > > Marek (internally), Geert and Alban reported errors like:
> > >         genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x68)
> > > The patchset fixes this issue.
> > >
> > > Tested on:
> > > 1. Exynos4412: Odroid U3,
> > > 2. Exynos5410: Odroid XU,
> > > 3. Exynos5422: Odroid XU3.
> > >  
> > 
> > Tested on Exynos5800 based peach-pi board, so fpr patch 06/10
> > 
> > Tested-by: Alim Akhtar <alim.akhtar@samsung.com>
> > 
> >   
> > > Other platforms not tested so testing would be highly appreciated.
> > >
> > > Best regards,
> > > Krzysztof
> > >
> > > Krzysztof Kozlowski (10):
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4210
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4x12
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4415
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos3250
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5250
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in
> > >     exynos5410/exynos542x
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5260
> > >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5440
> > >  
> > 
> > In general these patches looks good, feel free to add my review-by tag
> > for this series.
> > 
> > Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> >   
> 
> Thanks, I'll add your reviewed-by to all patches.

There is a couple of things that I don't like about these patches:

- Are you sure that all your interrupts are level? I appreciate that's
  a very common setting, but you seem to have simply decided that there
  wasn't any edge interrupt. Is that a "Let's see what breaks"
  situation? If so, It'd be good to state it.

- You've used the IRQ_TYPE_LEVEL_HIGH macro everywhere, but didn't use
  the GIC_SPI macro as the first argument of each interrupt specifier.
  I'm by no mean a fan of these macros, but you should at least have
  some consistency inside an interrupt specifier.

Thanks,

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

^ permalink raw reply

* [PATCH 5/6] arm/arm64: vgic-new: Implement VGICv3 CPU interface access
From: Marc Zyngier @ 2016-09-18  9:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CALicx6vp-0igFshdSvdoeSBvVg-+E_WWyYf0z=9A278sAs2sKQ@mail.gmail.com>

On Sun, 18 Sep 2016 12:00:01 +0530
Vijay Kilari <vijay.kilari@gmail.com> wrote:

> Hi Marc,
> 
> On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On 16/09/16 17:57, Vijay Kilari wrote:  
> >> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:  
> >>> On 16/09/16 13:20, vijay.kilari at gmail.com wrote:  
> >>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >>>>
> >>>> VGICv3 CPU interface registers are accessed using
> >>>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> >>>> as 64-bit. The cpu MPIDR value is passed along with register id.
> >>>> is used to identify the cpu for registers access.
> >>>>
> >>>> The version of VGIC v3 specification is define here
> >>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
> >>>>
> >>>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> >>>> ---
> >>>>  arch/arm64/include/uapi/asm/kvm.h   |   3 +
> >>>>  arch/arm64/kvm/Makefile             |   1 +
> >>>>  include/linux/irqchip/arm-gic-v3.h  |  30 ++++
> >>>>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 ++++
> >>>>  virt/kvm/arm/vgic/vgic-mmio-v3.c    |  18 +++
> >>>>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 ++++++++++++++++++++++++++++++++++++
> >>>>  virt/kvm/arm/vgic/vgic.h            |  10 ++
> >>>>  7 files changed, 385 insertions(+)  
> >
> > [...]
> >  
> >>>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >>>> new file mode 100644
> >>>> index 0000000..8e4f403
> >>>> --- /dev/null
> >>>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
> >>>> @@ -0,0 +1,296 @@
> >>>> +#include <linux/irqchip/arm-gic-v3.h>
> >>>> +#include <linux/kvm.h>
> >>>> +#include <linux/kvm_host.h>
> >>>> +#include <kvm/iodev.h>
> >>>> +#include <kvm/arm_vgic.h>
> >>>> +#include <asm/kvm_emulate.h>
> >>>> +#include <asm/kvm_arm.h>
> >>>> +#include <asm/kvm_mmu.h>
> >>>> +
> >>>> +#include "vgic.h"
> >>>> +#include "vgic-mmio.h"
> >>>> +#include "sys_regs.h"
> >>>> +
> >>>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                         const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +     u64 val;
> >>>> +     u32 ich_vtr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             val = p->regval;
> >>>> +             vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
> >>>> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
> >>>> +                           ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
> >>>> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
> >>>> +                          ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);  
> >>>
> >>> You've ignored my comments again: "What if userspace writes something
> >>> that is incompatible with the current configuration? Wrong number of ID
> >>> bits, or number of priorities?"  
> >>
> >> IMO, In case of incompatibility,
> >> If ID bits and PRI bits are less than HW supported, it is ok.  
> >
> > Yes. But you also need to track of what the guest has programmed in
> > order to be able to migrate it back to its original configuration.
> >  
> >> If ID bits and PRI bits are greater than HW supported, then warn would be good
> >> enough. Please suggest the behaviour that you think it should be.  
> >
> > No, it is an error, plain and simple. You cannot run in this condition.
> >  
> >>  
> >>>  
> >>>> +     } else {
> >>>> +             ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
> >>>> +
> >>>> +             val = 0;
> >>>> +             val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
> >>>> +                     ICH_VTR_PRI_BITS_SHIFT) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
> >>>> +             val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
> >>>> +                     ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
> >>>> +             val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
> >>>> +                     << ICC_CTLR_EL1_SEIS_SHIFT;
> >>>> +             val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
> >>>> +                     << ICC_CTLR_EL1_A3V_SHIFT;
> >>>> +             val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
> >>>> +                     ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
> >>>> +             val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
> >>>> +                     ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
> >>>> +
> >>>> +             p->regval = val;
> >>>> +     }
> >>>> +
> >>>> +     return true;
> >>>> +}
> >>>> +
> >>>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                        const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);
> >>>> +     } else {
> >>>> +             p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
> >>>> +     }
> >>>> +
> >>>> +     return true;
> >>>> +}
> >>>> +
> >>>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                         const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
> >>>> +                         ICC_BPR0_EL1_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);
> >>>> +     } else {
> >>>> +             p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
> >>>> +                          ICC_BPR0_EL1_MASK;
> >>>> +     }
> >>>> +
> >>>> +     return true;
> >>>> +}
> >>>> +
> >>>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                         const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
> >>>> +                          ICC_BPR1_EL1_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);
> >>>> +     } else {
> >>>> +             p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
> >>>> +                          ICC_BPR1_EL1_MASK;
> >>>> +     }
> >>>> +
> >>>> +     return true;
> >>>> +}
> >>>> +
> >>>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                           const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
> >>>> +                                   ICC_IGRPEN0_EL1_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);
> >>>> +     } else {
> >>>> +             p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
> >>>> +                          ICC_IGRPEN0_EL1_MASK;
> >>>> +     }
> >>>> +
> >>>> +     return true;
> >>>> +}
> >>>> +
> >>>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
> >>>> +                           const struct sys_reg_desc *r)
> >>>> +{
> >>>> +     struct vgic_vmcr vmcr;
> >>>> +
> >>>> +     vgic_get_vmcr(vcpu, &vmcr);
> >>>> +     if (p->is_write) {
> >>>> +             vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
> >>>> +                                   ICC_IGRPEN1_EL1_SHIFT;
> >>>> +             vgic_set_vmcr(vcpu, &vmcr);
> >>>> +     } else {
> >>>> +             p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
> >>>> +                          ICC_IGRPEN1_EL1_MASK;  
> >>>
> >>> From the previous review comments: "Shouldn't this account for the
> >>> ICC_CTLR_EL1.CBPR setting?"  
> >>
> >>  Ok. I think this comment is for ICC_BPR1_EL1 access.  
> >
> > Yes, sorry about the misplaced comment.
> >  
> >> I will make a check on ICC_CTLR.EL1.CBPR for accessing ICC_BPR1_EL1.  
> >
> > The reverse is also true: you also need to account the value of
> > ICC_BPR1_EL1 when accessing ICC_CTLR_EL1.
> >  
> 
>   ICC_CTLR_EL1 reg does not hold BPR1 value to account value of ICC_BPR1_EL1.

Oh please. What do you think ICC_CTLR_EL1.CPBR is for? It ties
ICC_BPR1_EL1 to ICC_BPR0_EL1. So if you have the following sequence:

	Set ICC_BPR0_EL1 to p0
	Set ICC_BPR1_EL1 to p1
	Set ICC_CTLR_EL1.CBPR to 1

you must ensure that ICC_BPR1_EL1 is now min(p0 + 1, 7), no matter what
p1 is. ICC_CTLR_EL1 may not hold the BPR1 value, but it directly
controls it.

Effectively, you must always consider BPR0, BPR1 and CBPR together.
Please read the architecture spec, it is one of the few things that are
easy actually to understand.

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

^ permalink raw reply

* [RFC PATCH 00/10] ARM: dts: exynos: Fix invalid GIC interrupt flags
From: Geert Uytterhoeven @ 2016-09-18  8:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474054971-16831-1-git-send-email-krzk@kernel.org>

Hi Krzysztof,

On Fri, Sep 16, 2016 at 9:42 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Marek (internally), Geert and Alban reported errors like:
>         genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x68)

I didn't report these on Exynos, though, so I think you should drop my
Reported-by.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

^ permalink raw reply

* [PATCH] dmaengine: sun6i: Add support for Allwinner A83T (sun8i) variant
From: Jean-Francois Moine @ 2016-09-18  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

The A83T SoC has the same dma engine as the A31 (sun6i), with a reduced
amount of endpoints and physical channels.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 Documentation/devicetree/bindings/dma/sun6i-dma.txt | 1 +
 drivers/dma/sun6i-dma.c                             | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/sun6i-dma.txt b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
index d13c136..6b26704 100644
--- a/Documentation/devicetree/bindings/dma/sun6i-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sun6i-dma.txt
@@ -7,6 +7,7 @@ Required properties:
 - compatible:	Must be one of
 		  "allwinner,sun6i-a31-dma"
 		  "allwinner,sun8i-a23-dma"
+		  "allwinner,sun8i-a83t-dma"
 		  "allwinner,sun8i-h3-dma"
 - reg:		Should contain the registers base address and length
 - interrupts:	Should contain a reference to the interrupt used by this device
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 3835fcd..8346199 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1011,6 +1011,12 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.nr_max_vchans   = 37,
 };
 
+static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
+	.nr_max_channels = 8,
+	.nr_max_requests = 28,
+	.nr_max_vchans   = 39,
+};
+
 /*
  * The H3 has 12 physical channels, a maximum DRQ port id of 27,
  * and a total of 34 usable source and destination endpoints.
@@ -1025,6 +1031,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 static const struct of_device_id sun6i_dma_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-dma", .data = &sun6i_a31_dma_cfg },
 	{ .compatible = "allwinner,sun8i-a23-dma", .data = &sun8i_a23_dma_cfg },
+	{ .compatible = "allwinner,sun8i-a83t-dma", .data = &sun8i_a83t_dma_cfg },
 	{ .compatible = "allwinner,sun8i-h3-dma", .data = &sun8i_h3_dma_cfg },
 	{ /* sentinel */ }
 };
-- 
2.10.0

^ permalink raw reply related

* [PATCH 5/6] arm/arm64: vgic-new: Implement VGICv3 CPU interface access
From: Vijay Kilari @ 2016-09-18  6:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57DC26C5.2060800@arm.com>

Hi Marc,

On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 16/09/16 17:57, Vijay Kilari wrote:
>> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> On 16/09/16 13:20, vijay.kilari at gmail.com wrote:
>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>
>>>> VGICv3 CPU interface registers are accessed using
>>>> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
>>>> as 64-bit. The cpu MPIDR value is passed along with register id.
>>>> is used to identify the cpu for registers access.
>>>>
>>>> The version of VGIC v3 specification is define here
>>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
>>>>
>>>> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>> ---
>>>>  arch/arm64/include/uapi/asm/kvm.h   |   3 +
>>>>  arch/arm64/kvm/Makefile             |   1 +
>>>>  include/linux/irqchip/arm-gic-v3.h  |  30 ++++
>>>>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 ++++
>>>>  virt/kvm/arm/vgic/vgic-mmio-v3.c    |  18 +++
>>>>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 ++++++++++++++++++++++++++++++++++++
>>>>  virt/kvm/arm/vgic/vgic.h            |  10 ++
>>>>  7 files changed, 385 insertions(+)
>
> [...]
>
>>>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>>> new file mode 100644
>>>> index 0000000..8e4f403
>>>> --- /dev/null
>>>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>>> @@ -0,0 +1,296 @@
>>>> +#include <linux/irqchip/arm-gic-v3.h>
>>>> +#include <linux/kvm.h>
>>>> +#include <linux/kvm_host.h>
>>>> +#include <kvm/iodev.h>
>>>> +#include <kvm/arm_vgic.h>
>>>> +#include <asm/kvm_emulate.h>
>>>> +#include <asm/kvm_arm.h>
>>>> +#include <asm/kvm_mmu.h>
>>>> +
>>>> +#include "vgic.h"
>>>> +#include "vgic-mmio.h"
>>>> +#include "sys_regs.h"
>>>> +
>>>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                         const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +     u64 val;
>>>> +     u32 ich_vtr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             val = p->regval;
>>>> +             vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>>>> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>>>> +                           ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
>>>> +             vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>>>> +                          ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>
>>> You've ignored my comments again: "What if userspace writes something
>>> that is incompatible with the current configuration? Wrong number of ID
>>> bits, or number of priorities?"
>>
>> IMO, In case of incompatibility,
>> If ID bits and PRI bits are less than HW supported, it is ok.
>
> Yes. But you also need to track of what the guest has programmed in
> order to be able to migrate it back to its original configuration.
>
>> If ID bits and PRI bits are greater than HW supported, then warn would be good
>> enough. Please suggest the behaviour that you think it should be.
>
> No, it is an error, plain and simple. You cannot run in this condition.
>
>>
>>>
>>>> +     } else {
>>>> +             ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>>>> +
>>>> +             val = 0;
>>>> +             val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
>>>> +                     ICH_VTR_PRI_BITS_SHIFT) << ICC_CTLR_EL1_PRI_BITS_SHIFT;
>>>> +             val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
>>>> +                     ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
>>>> +             val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
>>>> +                     << ICC_CTLR_EL1_SEIS_SHIFT;
>>>> +             val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
>>>> +                     << ICC_CTLR_EL1_A3V_SHIFT;
>>>> +             val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
>>>> +                     ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
>>>> +             val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
>>>> +                     ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
>>>> +
>>>> +             p->regval = val;
>>>> +     }
>>>> +
>>>> +     return true;
>>>> +}
>>>> +
>>>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                        const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> ICC_PMR_EL1_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>> +     } else {
>>>> +             p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & ICC_PMR_EL1_MASK;
>>>> +     }
>>>> +
>>>> +     return true;
>>>> +}
>>>> +
>>>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                         const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>>>> +                         ICC_BPR0_EL1_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>> +     } else {
>>>> +             p->regval = (vmcr.bpr << ICC_BPR0_EL1_SHIFT) &
>>>> +                          ICC_BPR0_EL1_MASK;
>>>> +     }
>>>> +
>>>> +     return true;
>>>> +}
>>>> +
>>>> +static bool access_gic_bpr1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                         const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             vmcr.abpr = (p->regval & ICC_BPR1_EL1_MASK) >>
>>>> +                          ICC_BPR1_EL1_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>> +     } else {
>>>> +             p->regval = (vmcr.abpr << ICC_BPR1_EL1_SHIFT) &
>>>> +                          ICC_BPR1_EL1_MASK;
>>>> +     }
>>>> +
>>>> +     return true;
>>>> +}
>>>> +
>>>> +static bool access_gic_grpen0(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                           const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             vmcr.grpen0 = (p->regval & ICC_IGRPEN0_EL1_MASK) >>
>>>> +                                   ICC_IGRPEN0_EL1_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>> +     } else {
>>>> +             p->regval = (vmcr.grpen0 << ICC_IGRPEN0_EL1_SHIFT) &
>>>> +                          ICC_IGRPEN0_EL1_MASK;
>>>> +     }
>>>> +
>>>> +     return true;
>>>> +}
>>>> +
>>>> +static bool access_gic_grpen1(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>>> +                           const struct sys_reg_desc *r)
>>>> +{
>>>> +     struct vgic_vmcr vmcr;
>>>> +
>>>> +     vgic_get_vmcr(vcpu, &vmcr);
>>>> +     if (p->is_write) {
>>>> +             vmcr.grpen1 = (p->regval & ICC_IGRPEN1_EL1_MASK) >>
>>>> +                                   ICC_IGRPEN1_EL1_SHIFT;
>>>> +             vgic_set_vmcr(vcpu, &vmcr);
>>>> +     } else {
>>>> +             p->regval = (vmcr.grpen1 << ICC_IGRPEN1_EL1_SHIFT) &
>>>> +                          ICC_IGRPEN1_EL1_MASK;
>>>
>>> From the previous review comments: "Shouldn't this account for the
>>> ICC_CTLR_EL1.CBPR setting?"
>>
>>  Ok. I think this comment is for ICC_BPR1_EL1 access.
>
> Yes, sorry about the misplaced comment.
>
>> I will make a check on ICC_CTLR.EL1.CBPR for accessing ICC_BPR1_EL1.
>
> The reverse is also true: you also need to account the value of
> ICC_BPR1_EL1 when accessing ICC_CTLR_EL1.
>

  ICC_CTLR_EL1 reg does not hold BPR1 value to account value of ICC_BPR1_EL1.

Regards
Vijay

^ permalink raw reply

* [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code
From: Tomasz Figa @ 2016-09-18  4:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57DDF2EE.5060807@rock-chips.com>

Hi Mark,

On Sun, Sep 18, 2016 at 10:50 AM, Mark yao <mark.yao@rock-chips.com> wrote:
> On 2016?09?14? 20:54, Tomasz Figa wrote:
>>
>> Current code implements prepare_fb and cleanup_fb callbacks only to
>> grab/release fb references, which is already done by atomic framework
>> when creating/destryoing plane state. Let's remove these
>> unused bits.
>>
>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>> ---
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------
>>   1 file changed, 18 deletions(-)
>
>
> Hi Tomasz
>
> I think we can't get rid of the prepare_fb and cleanup_fb

I think I have to disagree. Please see below for detailed explanation.

>
> see the reason:
> commit 44d0237a26395ac94160cf23f32769013b365590
> Author: Mark Yao <mark.yao@rock-chips.com>
> Date:   Fri Apr 29 11:37:20 2016 +0800
>
>     drm/rockchip: vop: fix iommu crash with async atomic
>
>     After async atomic_commit callback, drm_atomic_clean_old_fb will
>     clean all old fb, but because async, the old fb may be also on
>     the vop hardware, dma will access the old fb buffer, clean old
>     fb will cause iommu page fault.

I think the above is not quite right. Atomic plane state holds a
reference to its fb and old state is not supposed to be destroyed
until the flip completes.

Indeed current rockchip_atomic_commit implementation has following
order of calls: rockchip_atomic_wait_for_complete(),
drm_atomic_helper_cleanup_planes(), drm_atomic_state_free(). This
means that .cleanup_fb() is called from
drm_atomic_helper_cleanup_planes() just before drm_atomic_state_free()
will release references by destroying old plane states. Note that both
are called already after rockchip_atomic_wait_for_complete(), so it
should be already safe to free the old fbs.

So the above fix doesn't really do anything, possibly just covers the
race condition of the original wait for vblank function by delaying
drm_atomic_state_free() a bit.

Moreover, the whole series has been thoroughly tested in Chrome OS 4.4
kernel, including async commits. (There is still a possibility some
newer upstream changes slightly modified the semantics, but I couldn't
find such difference. Actually one of the advantages of atomic helpers
was to avoid manually refcounting the fbs from the driver.)

Best regards,
Tomasz

^ permalink raw reply

* [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang @ 2016-09-18  3:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5264074.nuyDhEuOR4@wuerfel>

Hi, Arnd,


On 2016?09?14? 22:23, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 10:16:28 PM CEST zhichang.yuan wrote:
>>>
>>> No need to guard includes with an #ifdef.
>> If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
>> extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
>  
> There is no problem with making declarations visible for functions that
> are not part of the kernel, we do that all the time.
> 
>>>> +#define BUILDS_RW(bwl, type)                                                \
>>>> +static inline void reads##bwl(const volatile void __iomem *addr,    \
>>>> +                            void *buffer, unsigned int count)       \
>>>> +{                                                                   \
>>>> +    if (count) {                                                    \
>>>> +            type *buf = buffer;                                     \
>>>> +                                                                    \
>>>> +            do {                                                    \
>>>> +                    type x = __raw_read##bwl(addr);                 \
>>>> +                    *buf++ = x;                                     \
>>>> +            } while (--count);                                      \
>>>> +    }                                                               \
>>>> +}                                                                   \
>>>> +                                                                    \
>>>> +static inline void writes##bwl(volatile void __iomem *addr,         \
>>>> +                            const void *buffer, unsigned int count) \
>>>> +{                                                                   \
>>>> +    if (count) {                                                    \
>>>> +            const type *buf = buffer;                               \
>>>> +                                                                    \
>>>> +            do {                                                    \
>>>> +                    __raw_write##bwl(*buf++, addr);                 \
>>>> +            } while (--count);                                      \
>>>> +    }                                                               \
>>>> +}
>>>> +
>>>> +BUILDS_RW(b, u8)
>>>
>>> Why is this in here?
>> the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
>> to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
>>
>> It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
>> those function needed here....
>>
>> Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
>>
>> #ifdef CONFIG_ARM64_INDIRECT_PIO
>> #define inb inb
>> extern u8 inb(unsigned long addr);
>>
>> #define outb outb
>> extern void outb(u8 value, unsigned long addr);
>>
>> #define insb insb
>> extern void insb(unsigned long addr, void *buffer, unsigned int count);
>>
>> #define outsb outsb
>> extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
>> #endif
>>
>> and definitions of all these functions are in extio.c :
>>
>> u8 inb(unsigned long addr)
>> {
>>         if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
>>                         arm64_extio_ops->end < addr)
>>                 return readb(PCI_IOBASE + addr);
>>         else
>>                 return arm64_extio_ops->pfin ?
>>                         arm64_extio_ops->pfin(arm64_extio_ops->devpara,
>>                                 addr + arm64_extio_ops->ptoffset, NULL,
>>                                 sizeof(u8), 1) : -1;
>> }
>> .....
> 
> Yes, sounds good.
> 
>>>> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>>>>  #define IO_SPACE_LIMIT              (PCI_IO_SIZE - 1)
>>>>  #define PCI_IOBASE          ((void __iomem *)PCI_IO_START)
>>>>  
>>>> +
>>>> +/*
>>>> + * redefine the in(s)b/out(s)b for indirect-IO.
>>>> + */
>>>> +#define inb inb
>>>> +static inline u8 inb(unsigned long addr)
>>>> +{
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> +    if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>>>> +                    addr <= arm64_extio_ops->end)
>>>> +            return extio_inb(addr);
>>>> +#endif
>>>> +    return readb(PCI_IOBASE + addr);
>>>> +}
>>>> +
>>>
>>> Looks ok, but you only seem to do this for the 8-bit
>>> accessors, when it should be done for 16-bit and 32-bit
>>> ones as well for consistency.
>> Hip06 LPC only support 8-bit I/O operations on the designated port.
> 
> That is an interesting limitation. Maybe still call the extio operations
> and have them do WARN_ON_ONCE() instead?
> 
> If you get a driver that calls inw/outw on the range that is owned
> by the LPC bus, you otherwise get an unhandled page fault in kernel
> space, which is not as nice.
> 
Yes. It probably cause kernel panic.
Will define the extio operations for other IO length and add the corresponding WARNINGS.

Best,
Zhichang


>>>> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
>>>> new file mode 100644
>>>> index 0000000..1e7a9c5
>>>> --- /dev/null
>>>> +++ b/drivers/bus/extio.c
>>>> @@ -0,0 +1,66 @@
>>>
>>> This is in a globally visible directory
>>>
>>>> +
>>>> +struct extio_ops *arm64_extio_ops;
>>>
>>> But the identifier uses an architecture specific prefix. Either
>>> move the whole file into arch/arm64, or make the naming so that
>>> it can be used for everything.
>>
>> I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
> 
> Ok, that simplifies it a lot, you can just do everything in asm/io.h then.
> 
> 	Arnd
> 

^ permalink raw reply

* [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode
From: Po Liu @ 2016-09-18  3:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160918005212.GF15478@tiger>

Hi Shawn,


>  -----Original Message-----
>  From: Shawn Guo [mailto:shawnguo at kernel.org]
>  Sent: Sunday, September 18, 2016 8:52 AM
>  To: Po Liu
>  Cc: linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
>  linux-kernel at vger.kernel.org; devicetree at vger.kernel.org; Roy Zang; Arnd
>  Bergmann; Marc Zyngier; Stuart Yoder; Leo Li; M.H. Lian; Murali
>  Karicheri; Bjorn Helgaas; Mingkai Hu
>  Subject: Re: [PATCH v5 3/3] pci:aer: add support aer interrupt with none
>  MSI/MSI-X/INTx mode
>  
>  On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
>  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
>  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
>  > maybe there is interrupt line for aer pme etc. Search the interrupt
>  > number in the fdt file. Then fixup the dev->irq with it.
>  >
>  > Signed-off-by: Po Liu <po.liu@nxp.com>
>  
>  Will the new kernel work with existing/old DTB?  I'm trying to
>  understand the dependency between driver and DTS changes.

Yes, We've never use name 'intr' before. So we remove it is ok. 
'aer' is a dts name for researching it's true interrupt number by kernel. This patch is first time to use name 'aer'. So it must be compatible with existing/old DTB.

>  
>  Shawn
>  
>  > ---
>  > changes for v5:
>  > 	- Add clear 'aer' interrup-names description
>  >
>  >  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
>  >  drivers/pci/pcie/portdrv_core.c                    | 31
>  +++++++++++++++++++---
>  >  2 files changed, 35 insertions(+), 7 deletions(-)
>  >
>  > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > index 41e9f55..101d0a7 100644
>  > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
>  > @@ -18,8 +18,10 @@ Required properties:
>  >  - reg: base addresses and lengths of the PCIe controller
>  >  - interrupts: A list of interrupt outputs of the controller. Must
>  contain an
>  >    entry for each entry in the interrupt-names property.
>  > -- interrupt-names: Must include the following entries:
>  > -  "intr": The interrupt that is asserted for controller interrupts
>  > +- interrupt-names: It may be include the following entries:
>  > +  "aer": The interrupt that is asserted for aer interrupt
>  > +  "pme": The interrupt that is asserted for pme interrupt
>  > +  ......
>  >  - fsl,pcie-scfg: Must include two entries.
>  >    The first entry must be a link to the SCFG device node
>  >    The second entry must be '0' or '1' based on physical PCIe
>  controller index.
>  > @@ -35,8 +37,9 @@ Example:
>  >  		reg = <0x00 0x03400000 0x0 0x00010000   /* controller
>  registers */
>  >  		       0x40 0x00000000 0x0 0x00002000>; /* configuration
>  space */
>  >  		reg-names = "regs", "config";
>  > -		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /*
>  controller interrupt */
>  > -		interrupt-names = "intr";
>  > +		interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, /* aer
>  interrupt */
>  > +			<GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* pme interrupt */
>  > +		interrupt-names = "aer", "pme";
>  >  		fsl,pcie-scfg = <&scfg 0>;
>  >  		#address-cells = <3>;
>  >  		#size-cells = <2>;
>  > diff --git a/drivers/pci/pcie/portdrv_core.c
>  > b/drivers/pci/pcie/portdrv_core.c index e9270b4..7c4943d 100644
>  > --- a/drivers/pci/pcie/portdrv_core.c
>  > +++ b/drivers/pci/pcie/portdrv_core.c
>  > @@ -16,6 +16,7 @@
>  >  #include <linux/slab.h>
>  >  #include <linux/pcieport_if.h>
>  >  #include <linux/aer.h>
>  > +#include <linux/of_irq.h>
>  >
>  >  #include "../pci.h"
>  >  #include "portdrv.h"
>  > @@ -200,6 +201,28 @@ static int pcie_port_enable_msix(struct pci_dev
>  > *dev, int *vectors, int mask)  static int init_service_irqs(struct
>  > pci_dev *dev, int *irqs, int mask)  {
>  >  	int i, irq = -1;
>  > +	int ret;
>  > +	struct device_node *np = NULL;
>  > +
>  > +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
>  > +		irqs[i] = 0;
>  > +
>  > +	if (dev->bus->dev.of_node)
>  > +		np = dev->bus->dev.of_node;
>  > +
>  > +	/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
>  > +	 * request irq for aer
>  > +	 */
>  > +	if (IS_ENABLED(CONFIG_OF_IRQ) && np &&
>  > +			(mask & PCIE_PORT_SERVICE_PME)) {
>  > +		ret = of_irq_get_byname(np, "aer");
>  > +		if (ret > 0) {
>  > +			irqs[PCIE_PORT_SERVICE_AER_SHIFT] = ret;
>  > +			if (dev->irq)
>  > +				irq = dev->irq;
>  > +			goto no_msi;
>  > +		}
>  > +	}
>  >
>  >  	/*
>  >  	 * If MSI cannot be used for PCIe PME or hotplug, we have to use
>  @@
>  > -225,11 +248,13 @@ static int init_service_irqs(struct pci_dev *dev,
>  int *irqs, int mask)
>  >  		irq = dev->irq;
>  >
>  >   no_msi:
>  > -	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
>  > -		irqs[i] = irq;
>  > +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
>  > +		if (!irqs[i])
>  > +			irqs[i] = irq;
>  > +	}
>  >  	irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
>  >
>  > -	if (irq < 0)
>  > +	if (irq < 0 && irqs[PCIE_PORT_SERVICE_AER_SHIFT] < 0)
>  >  		return -ENODEV;
>  >  	return 0;
>  >  }
>  > --
>  > 2.1.0.27.g96db324
>  >
>  >
>  > _______________________________________________
>  > linux-arm-kernel mailing list
>  > linux-arm-kernel at lists.infradead.org
>  > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [RESEND PATCH v4] thermal: tango: add resume support
From: Zhang Rui @ 2016-09-18  1:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57D96E5E.6070102@sigmadesigns.com>

On ?, 2016-09-14 at 17:35 +0200, Marc Gonzalez wrote:
> On 07/09/2016 10:55, Arnd Bergmann wrote:
> 
> > 
> > On Wednesday, September 7, 2016 10:30:41 AM CEST Marc Gonzalez
> > wrote:
> > 
> > > 
> > > When this platform is suspended, firmware powers the entire SoC
> > > down,
> > > except a few hardware blocks waiting for wakeup events. There is
> > > no
> > > context to save for this particular block.
> > > 
> > > Therefore, there is nothing useful for the driver to do on
> > > suspend;
> > > so we define a NULL suspend hook. On resume, the driver
> > > initializes
> > > the block exactly as is done in the probe callback.
> > > 
> > > Reviewed-by: Kevin Hilman <khilman@baylibre.com>
> > > Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Hello Rui, Eduardo,
> 
> The present patch has been reviewed by both Kevin Hilman and Arnd
> Bergmann,
> two arm-soc maintainers. Can you pick it up in time for inclusion in
> v4.9?

It has already been in thermal -release branch for a while, and queued
for 4.9.

thanks,
rui
> 
> Regards.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at??http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code
From: Mark yao @ 2016-09-18  1:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473857701-9250-3-git-send-email-tfiga@chromium.org>

On 2016?09?14? 20:54, Tomasz Figa wrote:
> Current code implements prepare_fb and cleanup_fb callbacks only to
> grab/release fb references, which is already done by atomic framework
> when creating/destryoing plane state. Let's remove these
> unused bits.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------
>   1 file changed, 18 deletions(-)

Hi Tomasz

I think we can't get rid of the prepare_fb and cleanup_fb

see the reason:
commit 44d0237a26395ac94160cf23f32769013b365590
Author: Mark Yao <mark.yao@rock-chips.com>
Date:   Fri Apr 29 11:37:20 2016 +0800

     drm/rockchip: vop: fix iommu crash with async atomic

     After async atomic_commit callback, drm_atomic_clean_old_fb will
     clean all old fb, but because async, the old fb may be also on
     the vop hardware, dma will access the old fb buffer, clean old
     fb will cause iommu page fault.

     Reference the fb and unreference it when the fb actuall swap out
     from vop hardware.


> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 7e811cf..68988c6 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -641,22 +641,6 @@ static void vop_plane_destroy(struct drm_plane *plane)
>   	drm_plane_cleanup(plane);
>   }
>   
> -static int vop_plane_prepare_fb(struct drm_plane *plane,
> -				struct drm_plane_state *new_state)
> -{
> -	if (plane->state->fb)
> -		drm_framebuffer_reference(plane->state->fb);
> -
> -	return 0;
> -}
> -
> -static void vop_plane_cleanup_fb(struct drm_plane *plane,
> -				 struct drm_plane_state *old_state)
> -{
> -	if (old_state->fb)
> -		drm_framebuffer_unreference(old_state->fb);
> -}
> -
>   static int vop_plane_atomic_check(struct drm_plane *plane,
>   			   struct drm_plane_state *state)
>   {
> @@ -849,8 +833,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>   }
>   
>   static const struct drm_plane_helper_funcs plane_helper_funcs = {
> -	.prepare_fb = vop_plane_prepare_fb,
> -	.cleanup_fb = vop_plane_cleanup_fb,
>   	.atomic_check = vop_plane_atomic_check,
>   	.atomic_update = vop_plane_atomic_update,
>   	.atomic_disable = vop_plane_atomic_disable,


-- 
?ark Yao

^ permalink raw reply

* [PATCH v2 3/4] arm64: dts: add Allwinner A64 SoC .dtsi
From: Icenowy Zheng @ 2016-09-18  1:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160917145121.GE17518@lukather>



17.09.2016, 22:56, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Thu, Sep 15, 2016 at 01:08:45AM +0100, Andr? Przywara wrote:
>> ?> --- a/MAINTAINERS
>> ?> +++ b/MAINTAINERS
>> ?> @@ -982,6 +982,7 @@ M: Chen-Yu Tsai <wens@csie.org>
>> ?> L: linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
>> ?> S: Maintained
>> ?> N: sun[x456789]i
>> ?> +F: arch/arm64/boot/dts/allwinner/
>>
>> ?If you promise to not break it needlessly ;-)
>
> We started doing so since 4.7, and we're already fighting needlessly
> with maintainers because of that.
>
>> ?> + cpus {
>> ?> + #address-cells = <1>;
>> ?> + #size-cells = <0>;
>> ?> +
>> ?> + cpu at 0 {
>>
>> ?This is probably me to blame here since I put you up to it, but you need
>> ?"cpux" names (e.g. "cpu0: cpu at 0 {") to match the ones that the PMU node
>> ?references below. dtc will refuse to compile it otherwise.
>
> Gaah, yes, of course.
>
>> ?> + i2c1_pins: i2c1_pins {
>> ?> + allwinner,pins = "PH2", "PH3";
>> ?> + allwinner,function = "i2c1";
>> ?> + allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> ?> + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> ?> + };
>> ?> +
>> ?> + uart0_pins_a: uart0 at 0 {
>> ?> + allwinner,pins = "PB8", "PB9";
>> ?> + allwinner,function = "uart0";
>> ?> + allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>> ?> + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
>> ?> + };
>>
>> ?So did I get this right that there is a strict "no user - no pins"
>> ?policy here?
>> ?I don't see a reason why we shouldn't have at least the other UART pins
>> ?described here as well - since they are a pure SoC property. That would
>> ?make it easier for people to enable them (with a simple, scripted fdt
>> ?one-liner command in U-Boot, for instance).
>
> To avoid having huge DT that are longer to load and parse, especially
> since every one wires it in the exact same way all the time. And the
> combination is just too high.
>
> On the A64, the UART0 can be exposed through PB9 and PF4 for TX, and
> PB8 and PF2 for RX. Even though the common usage would be to use PB8
> and PB9, or PF4 and PF6. But absolutely nothing prevents from using
> PB8 and PF4.
>
> You can then add CTS and RTS into the mix, and multiply that by the
> number of devices in the SoC.
>
>> ?I guess there will never be an official DT with more than 2 UARTs
>> ?enabled, although we have actually five UARTs on the headers on the
>> ?Pine64, for instance (and personally I am looking into using one as a
>> ?terminal server).
>
> We relaxed the rule lately however. Boards that have access to those
> UARTs on their headers can put a node in their DT with the right
> muxing filled already. Which brings you the simple, script fdt
> one-liner in U-Boot.
>
>> ?Also UART1 is connected to that WiFi/Bluetooth slot, having it enabled
>> ?should make BT work without further ado (but I haven't tested this).
>
> That's probably not true. You'll most likely need to enable a few
> regulators to have it working.

It seems that Andre has enabled the necessary regulator in the
newest version of his ARM Trusted Firmware...

>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
> ,
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode
From: Shawn Guo @ 2016-09-18  0:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473741659-17618-3-git-send-email-po.liu@nxp.com>

On Tue, Sep 13, 2016 at 12:40:59PM +0800, Po Liu wrote:
> On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
> When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
> maybe there is interrupt line for aer pme etc. Search the interrupt
> number in the fdt file. Then fixup the dev->irq with it.
> 
> Signed-off-by: Po Liu <po.liu@nxp.com>

Will the new kernel work with existing/old DTB?  I'm trying to
understand the dependency between driver and DTS changes.

Shawn

> ---
> changes for v5:
> 	- Add clear 'aer' interrup-names description
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt     | 11 +++++---
>  drivers/pci/pcie/portdrv_core.c                    | 31 +++++++++++++++++++---
>  2 files changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 41e9f55..101d0a7 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -18,8 +18,10 @@ Required properties:
>  - reg: base addresses and lengths of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> -- interrupt-names: Must include the following entries:
> -  "intr": The interrupt that is asserted for controller interrupts
> +- interrupt-names: It may be include the following entries:
> +  "aer": The interrupt that is asserted for aer interrupt
> +  "pme": The interrupt that is asserted for pme interrupt
> +  ......
>  - fsl,pcie-scfg: Must include two entries.
>    The first entry must be a link to the SCFG device node
>    The second entry must be '0' or '1' based on physical PCIe controller index.
> @@ -35,8 +37,9 @@ Example:
>  		reg = <0x00 0x03400000 0x0 0x00010000   /* controller registers */
>  		       0x40 0x00000000 0x0 0x00002000>; /* configuration space */
>  		reg-names = "regs", "config";
> -		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* controller interrupt */
> -		interrupt-names = "intr";
> +		interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>, /* aer interrupt */
> +			<GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>; /* pme interrupt */
> +		interrupt-names = "aer", "pme";
>  		fsl,pcie-scfg = <&scfg 0>;
>  		#address-cells = <3>;
>  		#size-cells = <2>;
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index e9270b4..7c4943d 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/pcieport_if.h>
>  #include <linux/aer.h>
> +#include <linux/of_irq.h>
>  
>  #include "../pci.h"
>  #include "portdrv.h"
> @@ -200,6 +201,28 @@ static int pcie_port_enable_msix(struct pci_dev *dev, int *vectors, int mask)
>  static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  {
>  	int i, irq = -1;
> +	int ret;
> +	struct device_node *np = NULL;
> +
> +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
> +		irqs[i] = 0;
> +
> +	if (dev->bus->dev.of_node)
> +		np = dev->bus->dev.of_node;
> +
> +	/* If root port doesn't support MSI/MSI-X/INTx in RC mode,
> +	 * request irq for aer
> +	 */
> +	if (IS_ENABLED(CONFIG_OF_IRQ) && np &&
> +			(mask & PCIE_PORT_SERVICE_PME)) {
> +		ret = of_irq_get_byname(np, "aer");
> +		if (ret > 0) {
> +			irqs[PCIE_PORT_SERVICE_AER_SHIFT] = ret;
> +			if (dev->irq)
> +				irq = dev->irq;
> +			goto no_msi;
> +		}
> +	}
>  
>  	/*
>  	 * If MSI cannot be used for PCIe PME or hotplug, we have to use
> @@ -225,11 +248,13 @@ static int init_service_irqs(struct pci_dev *dev, int *irqs, int mask)
>  		irq = dev->irq;
>  
>   no_msi:
> -	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++)
> -		irqs[i] = irq;
> +	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
> +		if (!irqs[i])
> +			irqs[i] = irq;
> +	}
>  	irqs[PCIE_PORT_SERVICE_VC_SHIFT] = -1;
>  
> -	if (irq < 0)
> +	if (irq < 0 && irqs[PCIE_PORT_SERVICE_AER_SHIFT] < 0)
>  		return -ENODEV;
>  	return 0;
>  }
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/1] ARM: imx5: Add clocks configuration
From: Shawn Guo @ 2016-09-18  0:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160918002159.GD15478@tiger>

On Sun, Sep 18, 2016 at 08:21:59AM +0800, Shawn Guo wrote:
> On Thu, Sep 15, 2016 at 08:42:33PM +0300, Alexander Shiyan wrote:
> > >???????, 15 ???????? 2016, 13:13 +03:00 ?? Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>:
> > >
> > >From: Kalle Kankare < kalle.kankare@vincit.fi >
> > >
> > >Add clocks configuration for CSI, FIRI and IEEE1588.
> > >
> > >Signed-off-by: Fabien Lahoudere < fabien.lahoudere@collabora.co.uk >
> > >---
> > >?drivers/clk/imx/clk-imx51-imx53.c      | 20 ++++++++++++++++++++
> > >?include/dt-bindings/clock/imx5-clock.h | 15 ++++++++++++++-
> > >?2 files changed, 34 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/clk/imx/clk-imx51-imx53.c b/drivers/clk/imx/clk-imx51-imx53.c
> > >index 29d4c44..1e3c9ea 100644
> > >--- a/drivers/clk/imx/clk-imx51-imx53.c
> > >+++ b/drivers/clk/imx/clk-imx51-imx53.c
> > >@@ -126,6 +126,7 @@ static const char *spdif0_com_sel[] = { "spdif0_podf", "ssi1_root_gate", };
> > >?static const char *mx51_spdif1_com_sel[] = { "spdif1_podf", "ssi2_root_gate", };
> > >?static const char *step_sels[] = { "lp_apm", };
> > >?static const char *cpu_podf_sels[] = { "pll1_sw", "step_sel" };
> > >+static const char *ieee1588_sels[] = { "pll3_sw", "pll4_sw", "dummy" /* usbphy2_clk */, "dummy" /* fec_phy_clk */ };
> > >?
> > >?static struct clk *clk[IMX5_CLK_END];
> > >?static struct clk_onecell_data clk_data;
> > >@@ -543,6 +544,25 @@ static void __init mx53_clocks_init(struct device_node *np)
> > ...
> > 
> > 2 Shawn Guo: Since the change affects only on i.MX53 architecture,
> > I would ask to change the subject to i.MX53 if it possible.
> 
> Yes.  Actually, since clock driver is not moved to drivers/clk, we

s/not/now

> shouldn't prefix it with "ARM: " any longer.

Shawn

^ permalink raw reply

* [PATCH 1/1] ARM: imx5: Add clocks configuration
From: Shawn Guo @ 2016-09-18  0:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473961353.68713919@f429.i.mail.ru>

On Thu, Sep 15, 2016 at 08:42:33PM +0300, Alexander Shiyan wrote:
> >???????, 15 ???????? 2016, 13:13 +03:00 ?? Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>:
> >
> >From: Kalle Kankare < kalle.kankare@vincit.fi >
> >
> >Add clocks configuration for CSI, FIRI and IEEE1588.
> >
> >Signed-off-by: Fabien Lahoudere < fabien.lahoudere@collabora.co.uk >
> >---
> >?drivers/clk/imx/clk-imx51-imx53.c      | 20 ++++++++++++++++++++
> >?include/dt-bindings/clock/imx5-clock.h | 15 ++++++++++++++-
> >?2 files changed, 34 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/clk/imx/clk-imx51-imx53.c b/drivers/clk/imx/clk-imx51-imx53.c
> >index 29d4c44..1e3c9ea 100644
> >--- a/drivers/clk/imx/clk-imx51-imx53.c
> >+++ b/drivers/clk/imx/clk-imx51-imx53.c
> >@@ -126,6 +126,7 @@ static const char *spdif0_com_sel[] = { "spdif0_podf", "ssi1_root_gate", };
> >?static const char *mx51_spdif1_com_sel[] = { "spdif1_podf", "ssi2_root_gate", };
> >?static const char *step_sels[] = { "lp_apm", };
> >?static const char *cpu_podf_sels[] = { "pll1_sw", "step_sel" };
> >+static const char *ieee1588_sels[] = { "pll3_sw", "pll4_sw", "dummy" /* usbphy2_clk */, "dummy" /* fec_phy_clk */ };
> >?
> >?static struct clk *clk[IMX5_CLK_END];
> >?static struct clk_onecell_data clk_data;
> >@@ -543,6 +544,25 @@ static void __init mx53_clocks_init(struct device_node *np)
> ...
> 
> 2 Shawn Guo: Since the change affects only on i.MX53 architecture,
> I would ask to change the subject to i.MX53 if it possible.

Yes.  Actually, since clock driver is not moved to drivers/clk, we
shouldn't prefix it with "ARM: " any longer.

Shawn

^ permalink raw reply

* [PATCH 2/2] clk: imx6: initialize GPU clocks
From: Shawn Guo @ 2016-09-18  0:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474017371-28966-2-git-send-email-l.stach@pengutronix.de>

On Fri, Sep 16, 2016 at 11:16:11AM +0200, Lucas Stach wrote:
> Initialize the GPU clock muxes to sane inputs. Until now they have
> not been changed from their default values, which means that both
> GPU3D shader and GPU2D core were fed by clock inputs whose rates
> exceed the maximium allowed frequency of the cores by as much as
> 200MHz.
> 
> This fixes a severe GPU stability issue on i.MX6DL.
> 
> Cc: stable at vger.kernel.org
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/clk/imx/clk-imx6q.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index 64c243173395..751c3e7d5843 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -633,6 +633,24 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  	if (IS_ENABLED(CONFIG_PCI_IMX6))
>  		clk_set_parent(clk[IMX6QDL_CLK_LVDS1_SEL], clk[IMX6QDL_CLK_SATA_REF_100M]);
>  
> +	/*
> +	 * Initialize the GPU clock muxes, so that the maximum specified clock
> +	 * rates for the respective SoC are not exceeded.
> +	 */
> +	if (clk_on_imx6dl()) {
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +	} else if (clk_on_imx6q()) {
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_MMDC_CH0_AXI]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU3D_SHADER_SEL],
> +		               clk[IMX6QDL_CLK_PLL2_PFD1_594M]);
> +		clk_set_parent(clk[IMX6QDL_CLK_GPU2D_CORE_SEL],
> +		               clk[IMX6QDL_CLK_PLL3_USB_OTG]);
> +	}
> +

Can we handle these with assigned-clock-parents from device tree?

Shawn

>  	imx_register_uart_clocks(uart_clks);
>  }
>  CLK_OF_DECLARE(imx6q, "fsl,imx6q-ccm", imx6q_clocks_init);
> -- 
> 2.8.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/2] clk: imx6: fix i.MX6DL clock tree to reflect reality
From: Shawn Guo @ 2016-09-18  0:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474017371-28966-1-git-send-email-l.stach@pengutronix.de>

Hi Anson,

Can you please help review/confirm the following clock tree changes
for i.MX6DL?  Thanks.

Shawn

On Fri, Sep 16, 2016 at 11:16:10AM +0200, Lucas Stach wrote:
> The current clock tree only implements the minimal set of differences
> between the i.MX6Q and the i.MX6DL, but that doesn't really reflect
> reality.
> 
> Apply the following fixes to match the RM:
> - DL has no GPU3D_SHADER_SEL/PODF, the shader domain is clocked by
>   GPU3D_CORE
> - GPU3D_SHADER_SEL/PODF has been repurposed as GPU2D_CORE_SEL/PODF
> - GPU2D_CORE_SEL/PODF has been repurposed as MLB_SEL/PODF
> 
> Cc: stable at vger.kernel.org
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/clk/imx/clk-imx6q.c               | 28 ++++++++++++++++------------
>  include/dt-bindings/clock/imx6qdl-clock.h |  4 +++-
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index ba1c1ae72ac2..64c243173395 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -318,11 +318,16 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  		clk[IMX6QDL_CLK_IPG_PER_SEL] = imx_clk_mux("ipg_per_sel", base + 0x1c, 6, 1, ipg_per_sels, ARRAY_SIZE(ipg_per_sels));
>  		clk[IMX6QDL_CLK_UART_SEL] = imx_clk_mux("uart_sel", base + 0x24, 6, 1, uart_sels, ARRAY_SIZE(uart_sels));
>  		clk[IMX6QDL_CLK_GPU2D_CORE_SEL] = imx_clk_mux("gpu2d_core_sel", base + 0x18, 16, 2, gpu2d_core_sels_2, ARRAY_SIZE(gpu2d_core_sels_2));
> +	} else if (clk_on_imx6dl()) {
> +		clk[IMX6QDL_CLK_MLB_SEL] = imx_clk_mux("mlb_sel",   base + 0x18, 16, 2, gpu2d_core_sels,   ARRAY_SIZE(gpu2d_core_sels));
>  	} else {
>  		clk[IMX6QDL_CLK_GPU2D_CORE_SEL] = imx_clk_mux("gpu2d_core_sel",   base + 0x18, 16, 2, gpu2d_core_sels,   ARRAY_SIZE(gpu2d_core_sels));
>  	}
>  	clk[IMX6QDL_CLK_GPU3D_CORE_SEL]   = imx_clk_mux("gpu3d_core_sel",   base + 0x18, 4,  2, gpu3d_core_sels,   ARRAY_SIZE(gpu3d_core_sels));
> -	clk[IMX6QDL_CLK_GPU3D_SHADER_SEL] = imx_clk_mux("gpu3d_shader_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
> +	if (clk_on_imx6dl())
> +		clk[IMX6QDL_CLK_GPU2D_CORE_SEL] = imx_clk_mux("gpu2d_core_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
> +	else
> +		clk[IMX6QDL_CLK_GPU3D_SHADER_SEL] = imx_clk_mux("gpu3d_shader_sel", base + 0x18, 8,  2, gpu3d_shader_sels, ARRAY_SIZE(gpu3d_shader_sels));
>  	clk[IMX6QDL_CLK_IPU1_SEL]         = imx_clk_mux("ipu1_sel",         base + 0x3c, 9,  2, ipu_sels,          ARRAY_SIZE(ipu_sels));
>  	clk[IMX6QDL_CLK_IPU2_SEL]         = imx_clk_mux("ipu2_sel",         base + 0x3c, 14, 2, ipu_sels,          ARRAY_SIZE(ipu_sels));
>  	clk[IMX6QDL_CLK_LDB_DI0_SEL]      = imx_clk_mux_flags("ldb_di0_sel", base + 0x2c, 9,  3, ldb_di_sels,      ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
> @@ -400,9 +405,15 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  		clk[IMX6QDL_CLK_LDB_DI0_DIV_3_5] = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7);
>  		clk[IMX6QDL_CLK_LDB_DI1_DIV_3_5] = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7);
>  	}
> -	clk[IMX6QDL_CLK_GPU2D_CORE_PODF]  = imx_clk_divider("gpu2d_core_podf",  "gpu2d_core_sel",    base + 0x18, 23, 3);
> +	if (clk_on_imx6dl())
> +		clk[IMX6QDL_CLK_MLB_PODF]  = imx_clk_divider("mlb_podf",  "mlb_sel",    base + 0x18, 23, 3);
> +	else
> +		clk[IMX6QDL_CLK_GPU2D_CORE_PODF]  = imx_clk_divider("gpu2d_core_podf",  "gpu2d_core_sel",    base + 0x18, 23, 3);
>  	clk[IMX6QDL_CLK_GPU3D_CORE_PODF]  = imx_clk_divider("gpu3d_core_podf",  "gpu3d_core_sel",    base + 0x18, 26, 3);
> -	clk[IMX6QDL_CLK_GPU3D_SHADER]     = imx_clk_divider("gpu3d_shader",     "gpu3d_shader_sel",  base + 0x18, 29, 3);
> +	if (clk_on_imx6dl())
> +		clk[IMX6QDL_CLK_GPU2D_CORE_PODF]  = imx_clk_divider("gpu2d_core_podf",     "gpu2d_core_sel",  base + 0x18, 29, 3);
> +	else
> +		clk[IMX6QDL_CLK_GPU3D_SHADER]     = imx_clk_divider("gpu3d_shader",     "gpu3d_shader_sel",  base + 0x18, 29, 3);
>  	clk[IMX6QDL_CLK_IPU1_PODF]        = imx_clk_divider("ipu1_podf",        "ipu1_sel",          base + 0x3c, 11, 3);
>  	clk[IMX6QDL_CLK_IPU2_PODF]        = imx_clk_divider("ipu2_podf",        "ipu2_sel",          base + 0x3c, 16, 3);
>  	clk[IMX6QDL_CLK_LDB_DI0_PODF]     = imx_clk_divider_flags("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1, 0);
> @@ -473,14 +484,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  	clk[IMX6QDL_CLK_ESAI_MEM]     = imx_clk_gate2_shared("esai_mem", "ahb",             base + 0x6c, 16, &share_count_esai);
>  	clk[IMX6QDL_CLK_GPT_IPG]      = imx_clk_gate2("gpt_ipg",       "ipg",               base + 0x6c, 20);
>  	clk[IMX6QDL_CLK_GPT_IPG_PER]  = imx_clk_gate2("gpt_ipg_per",   "ipg_per",           base + 0x6c, 22);
> -	if (clk_on_imx6dl())
> -		/*
> -		 * The multiplexer and divider of imx6q clock gpu3d_shader get
> -		 * redefined/reused as gpu2d_core_sel and gpu2d_core_podf on imx6dl.
> -		 */
> -		clk[IMX6QDL_CLK_GPU2D_CORE] = imx_clk_gate2("gpu2d_core", "gpu3d_shader", base + 0x6c, 24);
> -	else
> -		clk[IMX6QDL_CLK_GPU2D_CORE] = imx_clk_gate2("gpu2d_core", "gpu2d_core_podf", base + 0x6c, 24);
> +	clk[IMX6QDL_CLK_GPU2D_CORE] = imx_clk_gate2("gpu2d_core", "gpu2d_core_podf", base + 0x6c, 24);
>  	clk[IMX6QDL_CLK_GPU3D_CORE]   = imx_clk_gate2("gpu3d_core",    "gpu3d_core_podf",   base + 0x6c, 26);
>  	clk[IMX6QDL_CLK_HDMI_IAHB]    = imx_clk_gate2("hdmi_iahb",     "ahb",               base + 0x70, 0);
>  	clk[IMX6QDL_CLK_HDMI_ISFR]    = imx_clk_gate2("hdmi_isfr",     "video_27m",         base + 0x70, 4);
> @@ -511,7 +515,7 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
>  		 * The multiplexer and divider of the imx6q clock gpu2d get
>  		 * redefined/reused as mlb_sys_sel and mlb_sys_clk_podf on imx6dl.
>  		 */
> -		clk[IMX6QDL_CLK_MLB] = imx_clk_gate2("mlb",            "gpu2d_core_podf",   base + 0x74, 18);
> +		clk[IMX6QDL_CLK_MLB] = imx_clk_gate2("mlb",            "mlb_podf",   base + 0x74, 18);
>  	else
>  		clk[IMX6QDL_CLK_MLB] = imx_clk_gate2("mlb",            "axi",               base + 0x74, 18);
>  	clk[IMX6QDL_CLK_MMDC_CH0_AXI] = imx_clk_gate2("mmdc_ch0_axi",  "mmdc_ch0_axi_podf", base + 0x74, 20);
> diff --git a/include/dt-bindings/clock/imx6qdl-clock.h b/include/dt-bindings/clock/imx6qdl-clock.h
> index 29050337d9d5..da59fd9cdb5e 100644
> --- a/include/dt-bindings/clock/imx6qdl-clock.h
> +++ b/include/dt-bindings/clock/imx6qdl-clock.h
> @@ -269,6 +269,8 @@
>  #define IMX6QDL_CLK_PRG0_APB			256
>  #define IMX6QDL_CLK_PRG1_APB			257
>  #define IMX6QDL_CLK_PRE_AXI			258
> -#define IMX6QDL_CLK_END				259
> +#define IMX6QDL_CLK_MLB_SEL			259
> +#define IMX6QDL_CLK_MLB_PODF			260
> +#define IMX6QDL_CLK_END				261
>  
>  #endif /* __DT_BINDINGS_CLOCK_IMX6QDL_H */
> -- 
> 2.8.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH devicetree] ARM: BCM53573: Specify PMU and its ILP clock in the DT
From: Rafał Miłecki @ 2016-09-17 20:13 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

ILP clock (sometimes called a "slow clock") is a part of PMU (Power
Management Unit). There has been recently added a driver for it, so add
a proper entry in the DT as well.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
 arch/arm/boot/dts/bcm53573.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi
index efa07de..e2c496a 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -143,5 +143,17 @@
 		gmac1: ethernet at b000 {
 			reg = <0xb000 0x1000>;
 		};
+
+		pmu at 12000 {
+			compatible = "simple-mfd", "syscon";
+			reg = <0x00012000 0x00001000>;
+
+			ilp: ilp {
+				compatible = "brcm,bcm53573-ilp";
+				clocks = <&alp>;
+				#clock-cells = <0>;
+				clock-output-names = "ilp";
+			};
+		};
 	};
 };
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2] ARM: dts: exynos: Add entries for sound support on Odroid-XU board
From: Krzysztof Kozlowski @ 2016-09-17 20:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <71286ef8-4b7f-cfde-7bb9-9f3bd2289527@samsung.com>

On Fri, Sep 16, 2016 at 01:58:32PM +0200, Krzysztof Kozlowski wrote:
> On 09/16/2016 01:25 PM, Sylwester Nawrocki wrote:
> > On 09/16/2016 01:22 PM, Sylwester Nawrocki wrote:
> >> This patch adds device nodes for the AUDSS clock controller,
> >> peripheral DMA 0/1 controllers and the Audio Subsystem I2S controller.
> >> These entries are required for sound support on Odroid-XU board.
> >>
> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >> ---
> >> This patch depends on a patch adding clock ID macro definitions.
> >> I'm going to provide a topic branch containing required changes.
> >>
> >> Changes since v1:
> >>  - GIC_SPI, IRQ_TYPE_NONE used in the PDMA and max98080 interrupt
> >>    specifiers,
> >>  - assigned-clock-* properties moved to respective controller
> >>    nodes.
> > 
> > And here is a pull request containing clk dependency patches:
> > 
> > The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
> > 
> >   Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://linuxtv.org/snawrocki/samsung.git tags/clk-v4.9-exynos54x0-dt
> > 
> > for you to fetch changes up to 58d6506f327e3d192998ba03632f546da221b8d8:
> > 
> >   clk: samsung: exynos5410: Add clock IDs for PDMA and EPLL clocks (2016-09-09
> > 10:13:02 +0200)
> > 
> 
> Pulled and applied, thanks!
>

This does not boot...
http://www.krzk.eu/builders/boot-odroid-xu-exynos/builds/216
http://www.krzk.eu/builders/boot-odroid-xu-multi_v7/builds/195

I am going to send pull request this weekend (or Monday) so probably this
won't be included. If by any chance you prepare a fix soon, then it
will save me from rebasing the branch.

Best regards,
Krzysztof

^ permalink raw reply

* [RFC PATCH 00/10] ARM: dts: exynos: Fix invalid GIC interrupt flags
From: Krzysztof Kozlowski @ 2016-09-17 17:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAGOxZ50WiEVGNKf_1_8DQiKruxY=0813dko18fPfuRqxeyud0A@mail.gmail.com>

On Sat, Sep 17, 2016 at 11:05:39PM +0530, Alim Akhtar wrote:
> Hi Krzysztof,
> 
> On Sat, Sep 17, 2016 at 1:12 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > Hi,
> >
> > Marek (internally), Geert and Alban reported errors like:
> >         genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x68)
> > The patchset fixes this issue.
> >
> > Tested on:
> > 1. Exynos4412: Odroid U3,
> > 2. Exynos5410: Odroid XU,
> > 3. Exynos5422: Odroid XU3.
> >
> 
> Tested on Exynos5800 based peach-pi board, so fpr patch 06/10
> 
> Tested-by: Alim Akhtar <alim.akhtar@samsung.com>
> 
> 
> > Other platforms not tested so testing would be highly appreciated.
> >
> > Best regards,
> > Krzysztof
> >
> > Krzysztof Kozlowski (10):
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4210
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4x12
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4415
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos3250
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5250
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in
> >     exynos5410/exynos542x
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5260
> >   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5440
> >
> 
> In general these patches looks good, feel free to add my review-by tag
> for this series.
> 
> Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> 

Thanks, I'll add your reviewed-by to all patches.

Best regards,
Krzysztof

^ permalink raw reply

* [RFC PATCH 00/10] ARM: dts: exynos: Fix invalid GIC interrupt flags
From: Alim Akhtar @ 2016-09-17 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474054971-16831-1-git-send-email-krzk@kernel.org>

Hi Krzysztof,

On Sat, Sep 17, 2016 at 1:12 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Hi,
>
> Marek (internally), Geert and Alban reported errors like:
>         genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x68)
> The patchset fixes this issue.
>
> Tested on:
> 1. Exynos4412: Odroid U3,
> 2. Exynos5410: Odroid XU,
> 3. Exynos5422: Odroid XU3.
>

Tested on Exynos5800 based peach-pi board, so fpr patch 06/10

Tested-by: Alim Akhtar <alim.akhtar@samsung.com>


> Other platforms not tested so testing would be highly appreciated.
>
> Best regards,
> Krzysztof
>
> Krzysztof Kozlowski (10):
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4210
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4x12
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos4415
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos3250
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5250
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in
>     exynos5410/exynos542x
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5260
>   ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5440
>

In general these patches looks good, feel free to add my review-by tag
for this series.

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>


>  arch/arm/boot/dts/exynos3250-pinctrl.dtsi | 20 +++++--
>  arch/arm/boot/dts/exynos3250.dtsi         | 93 +++++++++++++++++-------------
>  arch/arm/boot/dts/exynos4.dtsi            | 94 ++++++++++++++++---------------
>  arch/arm/boot/dts/exynos4210-pinctrl.dtsi | 20 +++++--
>  arch/arm/boot/dts/exynos4210.dtsi         | 36 ++++++++----
>  arch/arm/boot/dts/exynos4415-pinctrl.dtsi | 20 +++++--
>  arch/arm/boot/dts/exynos4415.dtsi         | 92 +++++++++++++++++-------------
>  arch/arm/boot/dts/exynos4x12-pinctrl.dtsi | 20 +++++--
>  arch/arm/boot/dts/exynos4x12.dtsi         | 48 ++++++++++------
>  arch/arm/boot/dts/exynos5.dtsi            | 60 ++++++++++++++------
>  arch/arm/boot/dts/exynos5250.dtsi         | 80 +++++++++++++-------------
>  arch/arm/boot/dts/exynos5260.dtsi         | 39 ++++++++-----
>  arch/arm/boot/dts/exynos5410.dtsi         | 26 ++++-----
>  arch/arm/boot/dts/exynos5420.dtsi         | 78 ++++++++++++-------------
>  arch/arm/boot/dts/exynos5440.dtsi         | 48 ++++++++++------
>  arch/arm/boot/dts/exynos54xx.dtsi         | 34 +++++------
>  16 files changed, 485 insertions(+), 323 deletions(-)
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,
Alim

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Boris Brezillon @ 2016-09-17 17:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474125760-28168-1-git-send-email-sergio.prado@e-labworks.com>

Hi Sergio,

On Sat, 17 Sep 2016 12:22:40 -0300
Sergio Prado <sergio.prado@e-labworks.com> wrote:

> Tested on FriendlyARM Mini2440
> 
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
>  .../devicetree/bindings/mtd/samsung-s3c2410.txt    |  70 +++++++++++

DT maintainers usually ask people to keep the DT bindings doc in a
separate patch.

>  drivers/mtd/nand/s3c2410.c                         | 129 ++++++++++++++++++++-
>  include/linux/platform_data/mtd-nand-s3c2410.h     |   1 +
>  3 files changed, 195 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> new file mode 100644
> index 000000000000..1c39f6cf483b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> @@ -0,0 +1,70 @@
> +* Samsung S3C2410 and compatible NAND flash controller
> +
> +Required properties:
> +- compatible : The possible values are:
> +	"samsung,s3c2410-nand"
> +	"samsung,s3c2412-nand"
> +	"samsung,s3c2440-nand"
> +	"samsung,s3c6400-nand"
> +- reg : register's location and length.
> +- #address-cells, #size-cells : see nand.txt
> +- clocks : phandle to the nand controller clock
> +- clock-names : must contain "nand"
> +
> +Optional properties:
> +- samsung,tacls : time for active CLE/ALE to nWE/nOE
> +- samsung,twrph0 : active time for nWE/nOE
> +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive

Can you try to extract these information from the nand_sdr_timings
struct instead of passing it in your DT?

> +- samsung,ignore_unset_ecc : boolean to ignore error when we have
> +                             0xff,0xff,0xff read ECC, on the
> +                             assumption that it is an un-eccd page
> +
> +Optional children nodes:
> +Children nodes representing the available nand chips.
> +
> +Optional children properties:
> +- nand-ecc-mode : see nand.txt
> +- nand-on-flash-bbt : see nand.txt
> +
> +Each children device node may optionally contain a 'partitions' sub-node,
> +which further contains sub-nodes describing the flash partition mapping.
> +See partition.txt for more detail.
> +
> +Example:
> +
> +nand at 4e000000 {

s/nand/nand-controller/

> +	compatible = "samsung,s3c2440-nand";
> +	reg = <0x4e000000 0x40>;
> +
> +	#address-cells = <1>;
> +        #size-cells = <0>;
> +
> +	clocks = <&clocks HCLK_NAND>;
> +	clock-names = "nand";
> +
> +	samsung,tacls = <0>;
> +	samsung,twrph0 = <25>;
> +	samsung,twrph1 = <15>;

As said above, I think these timings can be extracted from the
nand_sdr_timings struct, which is know automatically attached to
nand_chip at detection time.

> +	samsung,ignore_unset_ecc;

Just discovered this ->ignore_unset_ecc property, and I don't
understand why it's here for...
Either you don't want to have ECC, and in this case you should set
NAND_ECC_NONE, or you want to have ECC calculated, and in this case,
the only valid situation where ECC bytes are 0xff is when the page is
erased.

If I'm right, please fix the driver instead of adding this DT property.
If I'm wrong, could you explain in more detail when you have ECC bytes
set to 0xff?

> +
> +	nand at 0 {

@0 implies having a reg property. I don't see any in your example, and
it's not document in the required property list.

Is your controller able to connect to different CS?

> +		nand-ecc-mode = "soft";
> +		nand-on-flash-bbt;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			partition at 0 {
> +				label = "u-boot";
> +				reg = <0 0x040000>;
> +			};
> +
> +			partition at 40000 {
> +				label = "kernel";
> +				reg = <0x040000 0x500000>;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index d9309cf0ce2e..ecbb9c9c1e9a 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -39,6 +39,8 @@
>  #include <linux/slab.h>
>  #include <linux/clk.h>
>  #include <linux/cpufreq.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
> @@ -185,6 +187,26 @@ struct s3c2410_nand_info {
>  #endif
>  };
>  
> +struct s3c24XX_nand_devtype_data {
> +	enum s3c_cpu_type type;
> +};
> +
> +struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = {
> +	.type = TYPE_S3C2410,
> +};
> +
> +struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = {
> +	.type = TYPE_S3C2412,
> +};
> +
> +struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = {
> +	.type = TYPE_S3C2440,
> +};
> +
> +struct s3c24XX_nand_devtype_data s3c6400_nand_devtype_data = {
> +	.type = TYPE_S3C2412,
> +};
> +
>  /* conversion functions */
>  
>  static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd)
> @@ -813,6 +835,8 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
>  	struct nand_chip *chip = &nmtd->chip;
>  	void __iomem *regs = info->regs;
>  
> +	nand_set_flash_node(chip, set->of_node);
> +
>  	chip->write_buf    = s3c2410_nand_write_buf;
>  	chip->read_buf     = s3c2410_nand_read_buf;
>  	chip->select_chip  = s3c2410_nand_select_chip;
> @@ -947,6 +971,96 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
>  	}
>  }
>  
> +#ifdef CONFIG_OF_MTD

Hm, I thought this symbol had been dropped, but apparently I forgot to
remove it. You should make it dependent on CONFIG_OF, not CONFIG_OF_MTD.

Anyway, I'm not even sure this ifdef is needed. Just test if
pdev->dev.of_node is NULL in s3c24xx_nand_probe_dt() and return -1 in
this case.

> +static const struct of_device_id s3c24xx_nand_dt_ids[] = {
> +	{
> +		.compatible = "samsung,s3c2410-nand",
> +		.data = &s3c2410_nand_devtype_data,
> +	}, {
> +		.compatible = "samsung,s3c2412-nand",
> +		.data = &s3c2412_nand_devtype_data,
> +	}, {
> +		.compatible = "samsung,s3c2440-nand",
> +		.data = &s3c2440_nand_devtype_data,
> +	}, {
> +		.compatible = "samsung,s3c6400-nand",
> +		.data = &s3c6400_nand_devtype_data,
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, s3c24xx_nand_dt_ids);
> +
> +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> +{
> +	const struct s3c24XX_nand_devtype_data *devtype_data;
> +	struct s3c2410_platform_nand *pdata;
> +	struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> +	struct device_node *np = pdev->dev.of_node, *child;
> +	const struct of_device_id *of_id;
> +	struct s3c2410_nand_set *sets;
> +
> +	of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev);
> +	if (!of_id)
> +		return 1;
> +
> +	devtype_data = of_id->data;
> +	info->cpu_type = devtype_data->type;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return -ENOMEM;
> +
> +	pdev->dev.platform_data = pdata;
> +
> +	of_property_read_u32(np, "samsung,tacls",  &pdata->tacls);
> +	of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0);
> +	of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1);
> +
> +	if (of_get_property(np, "samsung,ignore_unset_ecc", NULL))
> +		pdata->ignore_unset_ecc = 1;
> +
> +	pdata->nr_sets = of_get_child_count(np);
> +	if (!pdata->nr_sets)
> +		return 0;
> +
> +	sets = devm_kzalloc(&pdev->dev, sizeof(*sets) * pdata->nr_sets,
> +			   GFP_KERNEL);
> +	if (!sets)
> +		return -ENOMEM;
> +
> +	pdata->sets = sets;
> +
> +	for_each_available_child_of_node(np, child) {
> +
> +		sets->name = (char *)child->name;
> +		sets->of_node = child;
> +		sets->nr_chips = 1;
> +
> +		if (!of_property_match_string(child, "nand-ecc-mode", "none"))
> +			sets->disable_ecc = 1;
> +
> +		if (of_get_property(child, "nand-on-flash-bbt", NULL))
> +			sets->flash_bbt = 1;
> +

These properties are automatically extracted in nand_scan_ident(), why
do you need to parse them twice?

> +		sets++;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> +{
> +	return 1;
> +}
> +#endif
> +
> +static void s3c24xx_nand_probe_pdata(struct platform_device *pdev)
> +{
> +	struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> +
> +	info->cpu_type = platform_get_device_id(pdev)->driver_data;
> +}
> +
>  /* s3c24xx_nand_probe
>   *
>   * called by device layer when it finds a device matching
> @@ -956,8 +1070,7 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
>  */
>  static int s3c24xx_nand_probe(struct platform_device *pdev)
>  {
> -	struct s3c2410_platform_nand *plat = to_nand_plat(pdev);
> -	enum s3c_cpu_type cpu_type;
> +	struct s3c2410_platform_nand *plat;
>  	struct s3c2410_nand_info *info;
>  	struct s3c2410_nand_mtd *nmtd;
>  	struct s3c2410_nand_set *sets;
> @@ -967,8 +1080,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>  	int nr_sets;
>  	int setno;
>  
> -	cpu_type = platform_get_device_id(pdev)->driver_data;
> -
>  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
>  	if (info == NULL) {
>  		err = -ENOMEM;
> @@ -991,6 +1102,14 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>  
>  	s3c2410_nand_clk_set_state(info, CLOCK_ENABLE);
>  
> +	err = s3c24xx_nand_probe_dt(pdev);
> +	if (err > 0)
> +		s3c24xx_nand_probe_pdata(pdev);
> +	else if (err < 0)
> +		goto exit_error;
> +
> +	plat = to_nand_plat(pdev);
> +
>  	/* allocate and map the resource */
>  
>  	/* currently we assume we have the one resource */
> @@ -999,7 +1118,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
>  
>  	info->device	= &pdev->dev;
>  	info->platform	= plat;
> -	info->cpu_type	= cpu_type;
>  
>  	info->regs = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(info->regs)) {
> @@ -1156,6 +1274,7 @@ static struct platform_driver s3c24xx_nand_driver = {
>  	.id_table	= s3c24xx_driver_ids,
>  	.driver		= {
>  		.name	= "s3c24xx-nand",
> +		.of_match_table = s3c24xx_nand_dt_ids,

If you keep the #ifdef CONFIG_OF section

		.of_match_table = of_match_ptr(s3c24xx_nand_dt_ids),

>  	},
>  };
>  
> diff --git a/include/linux/platform_data/mtd-nand-s3c2410.h b/include/linux/platform_data/mtd-nand-s3c2410.h
> index c55e42ee57fa..9d20871e4bbd 100644
> --- a/include/linux/platform_data/mtd-nand-s3c2410.h
> +++ b/include/linux/platform_data/mtd-nand-s3c2410.h
> @@ -40,6 +40,7 @@ struct s3c2410_nand_set {
>  	char			*name;
>  	int			*nr_map;
>  	struct mtd_partition	*partitions;
> +	struct device_node	*of_node;
>  };
>  
>  struct s3c2410_platform_nand {

^ permalink raw reply

* [PATCH 06/10] ARM: dts: exynos: Fix invalid GIC interrupt flags in exynos5
From: Alim Akhtar @ 2016-09-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474054971-16831-7-git-send-email-krzk@kernel.org>

On Sat, Sep 17, 2016 at 1:12 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> Interrupt of type IRQ_TYPE_NONE is not allowed for GIC interrupts and
> generates an error:
>         genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x68)
>
> The GIC requires shared interrupts to be edge rising or level high.
> Platform declares support for both.  Choose level high everywhere.
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Reported-by: Alban Browaeys <alban.browaeys@gmail.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>

Tested in exynos5800, so

Tested-by: Alim Akhtar <alim.akhtar@samsung.com>

>  arch/arm/boot/dts/exynos5.dtsi | 60 ++++++++++++++++++++++++++++++------------
>  1 file changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
> index 8f06609879f5..d9b1607db5ad 100644
> --- a/arch/arm/boot/dts/exynos5.dtsi
> +++ b/arch/arm/boot/dts/exynos5.dtsi
> @@ -13,6 +13,7 @@
>   * published by the Free Software Foundation.
>   */
>
> +#include <dt-bindings/interrupt-controller/irq.h>
>  #include "exynos-syscon-restart.dtsi"
>
>  / {
> @@ -53,14 +54,38 @@
>                         interrupt-controller;
>                         samsung,combiner-nr = <32>;
>                         reg = <0x10440000 0x1000>;
> -                       interrupts =    <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> -                                       <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> -                                       <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> -                                       <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> -                                       <0 16 0>, <0 17 0>, <0 18 0>, <0 19 0>,
> -                                       <0 20 0>, <0 21 0>, <0 22 0>, <0 23 0>,
> -                                       <0 24 0>, <0 25 0>, <0 26 0>, <0 27 0>,
> -                                       <0 28 0>, <0 29 0>, <0 30 0>, <0 31 0>;
> +                       interrupts = <0 0 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 1 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 2 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 3 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 4 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 5 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 6 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 7 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 8 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 9 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 10 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 11 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 12 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 13 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 14 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 15 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 16 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 17 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 18 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 19 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 20 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 21 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 22 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 23 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 24 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 25 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 26 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 27 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 28 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 29 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 30 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 31 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
>                 gic: interrupt-controller at 10481000 {
> @@ -82,31 +107,31 @@
>                 serial_0: serial at 12C00000 {
>                         compatible = "samsung,exynos4210-uart";
>                         reg = <0x12C00000 0x100>;
> -                       interrupts = <0 51 0>;
> +                       interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
>                 serial_1: serial at 12C10000 {
>                         compatible = "samsung,exynos4210-uart";
>                         reg = <0x12C10000 0x100>;
> -                       interrupts = <0 52 0>;
> +                       interrupts = <0 52 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
>                 serial_2: serial at 12C20000 {
>                         compatible = "samsung,exynos4210-uart";
>                         reg = <0x12C20000 0x100>;
> -                       interrupts = <0 53 0>;
> +                       interrupts = <0 53 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
>                 serial_3: serial at 12C30000 {
>                         compatible = "samsung,exynos4210-uart";
>                         reg = <0x12C30000 0x100>;
> -                       interrupts = <0 54 0>;
> +                       interrupts = <0 54 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
>                 i2c_0: i2c at 12C60000 {
>                         compatible = "samsung,s3c2440-i2c";
>                         reg = <0x12C60000 0x100>;
> -                       interrupts = <0 56 0>;
> +                       interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         samsung,sysreg-phandle = <&sysreg_system_controller>;
> @@ -116,7 +141,7 @@
>                 i2c_1: i2c at 12C70000 {
>                         compatible = "samsung,s3c2440-i2c";
>                         reg = <0x12C70000 0x100>;
> -                       interrupts = <0 57 0>;
> +                       interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         samsung,sysreg-phandle = <&sysreg_system_controller>;
> @@ -126,7 +151,7 @@
>                 i2c_2: i2c at 12C80000 {
>                         compatible = "samsung,s3c2440-i2c";
>                         reg = <0x12C80000 0x100>;
> -                       interrupts = <0 58 0>;
> +                       interrupts = <0 58 IRQ_TYPE_LEVEL_HIGH>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         samsung,sysreg-phandle = <&sysreg_system_controller>;
> @@ -136,7 +161,7 @@
>                 i2c_3: i2c at 12C90000 {
>                         compatible = "samsung,s3c2440-i2c";
>                         reg = <0x12C90000 0x100>;
> -                       interrupts = <0 59 0>;
> +                       interrupts = <0 59 IRQ_TYPE_LEVEL_HIGH>;
>                         #address-cells = <1>;
>                         #size-cells = <0>;
>                         samsung,sysreg-phandle = <&sysreg_system_controller>;
> @@ -153,7 +178,8 @@
>                 rtc: rtc at 101E0000 {
>                         compatible = "samsung,s3c6410-rtc";
>                         reg = <0x101E0000 0x100>;
> -                       interrupts = <0 43 0>, <0 44 0>;
> +                       interrupts = <0 43 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <0 44 IRQ_TYPE_LEVEL_HIGH>;
>                         status = "disabled";
>                 };
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,
Alim

^ permalink raw reply


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