* [PATCH v7 7/9] drm/mediatek: add dsi transfer function
From: YT Shen @ 2016-09-12 10:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473215632.11736.21.camel@mtksdaap41>
Hi CK,
On Wed, 2016-09-07 at 10:33 +0800, CK Hu wrote:
> Hi, YT:
>
> On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote:
> > From: shaoming chen <shaoming.chen@mediatek.com>
> >
> > add dsi read/write commands for transfer function
> >
> > Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 188 +++++++++++++++++++++++++++++++++++++
> > 1 file changed, 188 insertions(+)
> >
>
> [snip...]
>
> >
> > +static void mtk_dsi_irq_data_clear(struct mtk_dsi *dsi, u32 irq_bit)
> > +{
> > + dsi->irq_data &= ~irq_bit;
> > +}
> > +
>
> [snip...]
>
> > +
> > +static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
> > + unsigned int timeout)
> > +{
> > + s32 ret = 0;
> > + unsigned long jiffies = msecs_to_jiffies(timeout);
> > +
> > + ret = wait_event_interruptible_timeout(_dsi_irq_wait_queue,
> > + dsi->irq_data & irq_flag,
> > + jiffies);
> > + if (ret == 0) {
> > + dev_info(dsi->dev, "Wait DSI IRQ(0x%08x) Timeout\n", irq_flag);
> > +
> > + mtk_dsi_enable(dsi);
> > + mtk_dsi_reset_engine(dsi);
> > + }
> > +
> > + return ret;
> > +}
>
> I think mtk_dsi_irq_data_clear() and mtk_dsi_wait_for_irq_done() should
> be moved to the 6th patch [1] of this series because these two functions
> deal the irq control.
We will move mtk_dsi_irq_data_clear() to patch "drm/mediatek: add dsi
interrupt control" and put mtk_dsi_wait_for_irq_done() here, because it
is used in the transfer function.
Regards,
yt.shen
>
>
> [1] https://patchwork.kernel.org/patch/9310819/
>
>
> Regards,
> CK
>
^ permalink raw reply
* [PATCH v7 6/9] drm/mediatek: add dsi interrupt control
From: YT Shen @ 2016-09-12 10:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473212348.11736.16.camel@mtksdaap41>
Hi CK,
On Wed, 2016-09-07 at 09:39 +0800, CK Hu wrote:
> Hi, YT:
>
> On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote:
> > From: shaoming chen <shaoming.chen@mediatek.com>
> >
> > add dsi interrupt control
> >
> > Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> > ---
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 76 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> >
>
> [snip...]
>
> >
> > +static wait_queue_head_t _dsi_irq_wait_queue;
>
> I think it's better to move this global variable into platform driver
> data. Maybe one day you have two dsi device and one global variable is
> not enough.
OK, we will move _dsi_irq_wait_queue into platform driver data.
>
> > +
>
> [snip...]
>
> > +
> > +static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
> > +{
> > + struct mtk_dsi *dsi = dev_id;
> > + u32 status, tmp;
> > + u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
> > +
> > + status = readl(dsi->regs + DSI_INTSTA);
>
> If you define as
>
> status = readl(dsi->regs + DSI_INTSTA) & flag;
>
> You can remove 'flag' in below statements and reduce code size.
Will do.
>
> > +
> > + if (status & flag) {
> > + do {
> > + mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
> > + tmp = readl(dsi->regs + DSI_INTSTA);
> > + } while (tmp & DSI_BUSY);
> > +
> > + mtk_dsi_mask(dsi, DSI_INTSTA, status & flag, 0);
> > + mtk_dsi_irq_data_set(dsi, status & flag);
> > + wake_up_interruptible(&_dsi_irq_wait_queue);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
>
> [snip...]
>
> >
> > @@ -869,8 +926,27 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > + irq_num = platform_get_irq(pdev, 0);
> > + if (irq_num < 0) {
> > + dev_err(&pdev->dev, "failed to request dsi irq resource\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
> > + ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
> > + IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
> > + return -EPROBE_DEFER;
> > + }
> > +
> > + dsi->irq_data = 0;
>
> You use devm_kzalloc() to allocate 'dsi', so this statement is
> redundant.
Will remove.
Regards,
yt.shen
>
> > + dev_info(dev, "dsi irq num is 0x%x\n", irq_num);
> > +
> > platform_set_drvdata(pdev, dsi);
> >
> > + init_waitqueue_head(&_dsi_irq_wait_queue);
> > +
> > return component_add(&pdev->dev, &mtk_dsi_component_ops);
> > }
>
>
> Regards,
> CK
>
>
^ permalink raw reply
* [PATCH v7 8/9] drm/mediatek: update DSI sub driver flow
From: YT Shen @ 2016-09-12 10:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473224283.11736.27.camel@mtksdaap41>
Hi CK,
On Wed, 2016-09-07 at 12:58 +0800, CK Hu wrote:
> Hi, YT:
>
> On Fri, 2016-09-02 at 19:24 +0800, YT Shen wrote:
> > This patch update enable/disable flow of DSI module and MIPI TX module
> >
> > Signed-off-by: shaoming chen <shaoming.chen@mediatek.com>
> > Signed-off-by: YT Shen <yt.shen@mediatek.com>
> > ---
>
> I think the description is too simple. Please briefly describe WHY of
> this patch. The original enable/disable flow is workable, so why do you
> need this patch? Without this patch, what problem would happen?
Got it, we will update more descriptions in the next version.
There is no transfer/interrupt function in the upstream DSI driver.
We also implement the following function [1][2] in this patch series.
Original flow works on there is a bridge chip: DSI -> bridge -> panel.
In this case: DSI -> panel, the DSI sub driver flow should be updated.
We need to initialize DSI first so that we can send commands to panel.
[1] https://patchwork.kernel.org/patch/9310819/
drm/mediatek: add dsi interrupt control
[2] https://patchwork.kernel.org/patch/9310823/
drm/mediatek: add dsi transfer function
>
> Regards,
> CK
>
>
^ permalink raw reply
* [PATCH v2 4/4] arm64: dts: add Pine64 support
From: Andre Przywara @ 2016-09-12 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v65JZ7gnOZ_LkkvGv5-bqXeK1E1O3eno85zYBSndNpN=Qw@mail.gmail.com>
Hi,
On 10/09/16 03:33, Chen-Yu Tsai wrote:
> Hi,
>
> On Sat, Sep 10, 2016 at 4:10 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> From: Andre Przywara <andre.przywara@arm.com>
>>
>> The Pine64 is a cost-efficient development board based on the
>> Allwinner A64 SoC.
>> There are three models: the basic version with Fast Ethernet and
>> 512 MB of DRAM (Pine64) and two Pine64+ versions, which both
>> feature Gigabit Ethernet and additional connectors for touchscreens
>> and a camera. Or as my son put it: "Those are smaller and these are
>> missing." ;-)
>> The two Pine64+ models just differ in the amount of DRAM
>> (1GB vs. 2GB). Since U-Boot will figure out the right size for us and
>> patches the DT accordingly we just need to provide one DT for the
>> Pine64+.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> [Maxime: Removed the common DTSI and include directly the pine64 DTS]
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>> arch/arm64/boot/dts/Makefile | 1 +
>> arch/arm64/boot/dts/allwinner/Makefile | 5 ++
>> .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts | 50 ++++++++++++++++
>> .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 70 ++++++++++++++++++++++
>> 4 files changed, 126 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/allwinner/Makefile
>> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>> create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>>
>> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
>> index 6e199c903676..ddcbf5a2c17e 100644
>> --- a/arch/arm64/boot/dts/Makefile
>> +++ b/arch/arm64/boot/dts/Makefile
>> @@ -1,4 +1,5 @@
>> dts-dirs += al
>> +dts-dirs += allwinner
>> dts-dirs += altera
>> dts-dirs += amd
>> dts-dirs += amlogic
>> diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
>> new file mode 100644
>> index 000000000000..1e29a5ae8282
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/allwinner/Makefile
>> @@ -0,0 +1,5 @@
>> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-pine64-plus.dtb sun50i-a64-pine64.dtb
>> +
>> +always := $(dtb-y)
>> +subdir-y := $(dts-dirs)
>> +clean-files := *.dtb
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>> new file mode 100644
>> index 000000000000..790d14daaa6a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
>> @@ -0,0 +1,50 @@
>> +/*
>> + * Copyright (c) 2016 ARM Ltd.
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + * a) This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Or, alternatively,
>> + *
>> + * b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include "sun50i-a64-pine64.dts"
>> +
>> +/ {
>> + model = "Pine64+";
>> + compatible = "pine64,pine64-plus", "allwinner,sun50i-a64";
>> +
>> + /* TODO: Camera, Ethernet PHY, touchscreen, etc. */
>> +};
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>> new file mode 100644
>> index 000000000000..da9bca51f5a9
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
>> @@ -0,0 +1,70 @@
>> +/*
>> + * Copyright (c) 2016 ARM Ltd.
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + * a) This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Or, alternatively,
>> + *
>> + * b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "sun50i-a64.dtsi"
>> +
>> +/ {
>> + model = "Pine64";
>> + compatible = "pine64,pine64", "allwinner,sun50i-a64";
>> +
>> + aliases {
>> + serial0 = &uart0;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> +};
>> +
>> +&uart0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart0_pins_a>;
>> + status = "okay";
>> +};
>> +
>> +&i2c1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&i2c1_pins>;
>> + status = "okay";
>
> Schematics say this is missing an external pull-up. Has anyone tried it?
> Without a pull-up any access on the i2c bus should just block.
I tried it a while ago (with an older kernel), though I had an external
pull-up on the device side, I think. I can give it a try again with
Maxime's branch once I find this I2C test device in one of my moving
boxes ;-)
Do other boards providing a Raspi-compatible header have an on-board
pull-up here? So will (some) hardware extensions for the Raspi fail on
the Pine64?
> Also this is on the RPi-2 compatible header. There's also an UART and SPI
> that are standard for the RPi-2 header. We should consider enabling them
> as well.
Interesting topic ;-)
As both interfaces can be configured for GPIO as well, I think Maxime
voted to not declare them as special function in the upstream DT.
UART0 is used for the console, so I guess some people may decide to use
those UART2 header pins for GPIO. Similarly for SPI: if you don't need
it, you get four (or five) additional GPIO.
A kind of related issue arises for those BT/Wifi headers. 99.9% of the
users will plug the Pine64 BT/WiFi module in there, at which point we
could declare MMC1 as SDIO and UART1 as enabled to cover this.
But then again nothing prevents people from building a custom module
which uses those pins for something else. Port G at least does not
multiplex any other IP to those pins - apart from GPIO, of course.
So what is the exact policy here? In the end I think any pin (including
UART0's PB8 and PB9) can be configured as GPIO, so do we make some
assumptions about "sensible" or predominant usage?
Does "# echo $DEVICE > /sys/devices/platform/$DEVICE/driver/unbind" work
to get those pins free later in case one wants to configure them as
GPIOs? In this case we may consider being more generous in declaring
common use cases in the upstream DT for the sake of the majority of users.
Cheers,
Andre.
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Maxime Ripard @ 2016-09-12 10:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912095923.GD1873@dell>
On Mon, Sep 12, 2016 at 10:59:23AM +0100, Lee Jones wrote:
> > >> +static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
> > >> + {
> > >> + .compatible = "allwinner,sun4i-a10-ts",
> > >> + .data = &sun4i_gpadc_mfd_cells,
> > >> + }, {
> > >> + .compatible = "allwinner,sun5i-a13-ts",
> > >> + .data = &sun5i_gpadc_mfd_cells,
> > >> + }, {
> > >> + .compatible = "allwinner,sun6i-a31-ts",
> > >> + .data = &sun6i_gpadc_mfd_cells,
> > >> + }, { /* sentinel */ }
> > >> +};
> > >
> > > Don't mix OF and MFD functionality.
> > >
> > > Why don't you create a node for "iio_hwmon" and have
> > > platform_of_populate() do your bidding?
> > >
> >
> > We are using a stable binding which we cannot modify. This means, the DT
> > in its current state can only be modified to add features, which is not
> > the case of this driver (it is a rewriting of an existing driver which
> > uses the rtp node).
>
> Then use .data = <defined model ID> and set up a switch() in .probe().
Uh? Why? It just adds a non-standard indirection, while using
of_match_device is very standard, and used extensively in Linux.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160912/bda7da40/attachment.sig>
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Lee Jones @ 2016-09-12 9:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <93bd339b-85ab-d0fc-5e80-e2aca290c0d7@free-electrons.com>
On Mon, 12 Sep 2016, Quentin Schulz wrote:
> On 12/09/2016 11:18, Lee Jones wrote:
> > On Thu, 08 Sep 2016, Quentin Schulz wrote:
> >
> [...]
[...]
> >> +++ b/drivers/mfd/sun4i-gpadc-mfd.c
[...]
> >> +static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
> >> + {
> >> + .name = "sun4i-a10-gpadc-iio",
> >> + .resources = adc_resources,
> >> + .num_resources = ARRAY_SIZE(adc_resources),
> >> + }, {
> >> + .name = "iio_hwmon",
> >> + }
> >
> > Single line please
> >
> > { .name = "iio_hwmon" }
> >
>
> + {
> + .name = "sun4i-a10-gpadc-iio",
> + .resources = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> + }, { .name = "iio_hwmon" }
>
> or
>
> + {
> + .name = "sun4i-a10-gpadc-iio",
> + .resources = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> + },
> + { .name = "iio_hwmon" }
>
> ?
The latter.
[...]
> >> +static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
> >> + {
> >> + .compatible = "allwinner,sun4i-a10-ts",
> >> + .data = &sun4i_gpadc_mfd_cells,
> >> + }, {
> >> + .compatible = "allwinner,sun5i-a13-ts",
> >> + .data = &sun5i_gpadc_mfd_cells,
> >> + }, {
> >> + .compatible = "allwinner,sun6i-a31-ts",
> >> + .data = &sun6i_gpadc_mfd_cells,
> >> + }, { /* sentinel */ }
> >> +};
> >
> > Don't mix OF and MFD functionality.
> >
> > Why don't you create a node for "iio_hwmon" and have
> > platform_of_populate() do your bidding?
> >
>
> We are using a stable binding which we cannot modify. This means, the DT
> in its current state can only be modified to add features, which is not
> the case of this driver (it is a rewriting of an existing driver which
> uses the rtp node).
Then use .data = <defined model ID> and set up a switch() in .probe().
> >> +static int sun4i_gpadc_mfd_probe(struct platform_device *pdev)
> >
> > Remove all mention of "mfd" from this file.
> >
> > (Accept the calls to the MFD API of course).
> >
> [...]
> >> +
> >> +MODULE_DEVICE_TABLE(of, sun4i_gpadc_mfd_of_match);
> >
> > Place this directly under the table.
> >
> >> +static struct platform_driver sun4i_gpadc_mfd_driver = {
> >> + .driver = {
> >> + .name = "sun4i-adc-mfd",
> >> + .of_match_table = of_match_ptr(sun4i_gpadc_mfd_of_match),
> >> + },
> >> + .probe = sun4i_gpadc_mfd_probe,
> >
> > No .remove?
> >
>
> No, everything in probe is handled with devm functions.
Don't you need to undo the register write you did?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 0/7] drm/sun4i: Introduce A33 display driver
From: Maxime Ripard @ 2016-09-12 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v660je7FAiXfeRb5Ynwtr8=fYDR1JHzrj2Hk+43_J0XRLg@mail.gmail.com>
Hi,
On Wed, Sep 07, 2016 at 12:49:58PM +0800, Chen-Yu Tsai wrote:
> On Wed, Sep 7, 2016 at 2:54 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Tue, Sep 06, 2016 at 10:50:09AM +0800, Chen-Yu Tsai wrote:
> >> >> The implementation might be along the lines of
> >> >>
> >> >> 1. having multiple output ports, each for a different interface type.
> >> >> (Some platforms go this route)
> >> >>
> >> >> Or
> >> >>
> >> >> 2. having a DT property describe what the output interface is.
> >> >>
> >> >> The RGB/TCON driver would then setup the registers accordingly.
> >> >
> >> > Hmmm, yeah, we would need to adjust the bindings too...
> >> >
> >> > I guess I'd prefer 1), but that would also be the most invasive
> >> > solution. I'm not sure how the DT maintainers feel about that.
> >>
> >> I wonder if the TCON could use its 2 channels simultaneously?
> >
> > No, it's mutually exclusive.
>
> I don't see how though. Are you referring to the IO_Map_Sel bit?
Yes.
> I assume that only controls the external output pins though.
As far as I know, channel 1 has no external pins, it's always one of
the blocks using the channel 1 that have external pins.
> >> Like output to one LCD, then mirror through HDMI/VGA?
> >> The first option would be able to cover this better?
> >
> > Even if it wasn't exclusive, that wouldn't be possible
> > unfortunately. Or rather, this would be possible if the LCD and the
> > HDMI screen had the same timings, which is very unlikely.
>
> What about an LCD-VGA bridge + HDMI in mirror mode on sun6i?
> That should work.
The same resolution doesn't mean you have the same timings. The
porches and sync length might be different, in which case you'll have
to have a different pixel clock and different register set ups.
> >> In addition we'll have to rework the TV encoder binding as well.
> >>
> >> The 2 TV encoders (on the A20) each have four DACs, which map
> >> onto 4 external pins. The address space includes a not so easy
> >> to use mux. More importantly, the binding needs to specify which
> >> pin is used for what signal (RGB, YUV, S/Video, composite).
> >> There seems to be an implicit rule that 1 pin is always used
> >> for composite, and the 3 others RGB, though.
> >
> > I'm not sure why we would need to rework this one though. We have no
> > way to detect whether the screen is connected or not on either
> > connectors, and we can't have both output running at the same time for
> > the same reason than mention above.
>
> I would like to add connector nodes. At least we can specify stuff
> like what DAC outputs are used for which connector, what type of
> connector, and a ddc bus for VGA connectors. I haven't worked out
> the details though.
The DAC really don't matter. It's purely internal to the driver
itself, it doesn't change from one SoC to the other, so it doesn't
really make sense to have it in the DT.
And is there any design that uses an i2c bus for DDC together with the
TV Encoder?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160912/ff391da2/attachment.sig>
^ permalink raw reply
* [PATCHv2 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)
From: Alexandre Belloni @ 2016-09-12 9:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912094733.21501-4-richard.genoud@gmail.com>
On 12/09/2016 at 11:47:33 +0200, Richard Genoud wrote :
> Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
> hardware handshake is enabled") broke the hardware handshake on SAM9x5
> platforms.
>
> On Atmel platforms, the USART can only handle the handware handshake
> (ATMEL_US_USMODE_HWHS) if FIFOs or PDC are used.
>
> Thus, ATMEL_US_USMODE_HWHS mode should only be used in this case.
>
> For SAM9x5, there's no FIFOs nor PDC for the USART, so the mode should
> be ATMEL_US_USMODE_NORMAL and the RTS pin should be controlled by the
> driver.
>
> NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
>
> Tested on SAM9G35-CM with and without DMA
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
> ---
> drivers/tty/serial/atmel_serial.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index e9b4fbf88c2d..32154e7231ce 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2130,15 +2130,19 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> } else if ((termios->c_cflag & CRTSCTS) &&
> !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
> /*
> - * RS232 with hardware handshake (RTS/CTS)
> - * handled by the controller.
> + * Automatic hardware handshake (RTS/CTS) only work with
> + * FIFOs or PDC.
> + * Meaning that on SAM9x5 the controller can't handle
> + * the hardware handshake (no FIFOs nor PDC on these platforms).
> */
> - if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
> - dev_info(port->dev, "not enabling hardware flow control because DMA is used");
> - termios->c_cflag &= ~CRTSCTS;
> - } else {
> + if (atmel_use_pdc_rx(port) || atmel_use_fifo(port))
> mode |= ATMEL_US_USMODE_HWHS;
> - }
> + else
> + /*
> + * The hardware handshake won't be handle by the
> + * controller but by the driver.
> + */
> + mode |= ATMEL_US_USMODE_NORMAL;
You still need the case where HWHS is impossible and there are no gpio
configured. You need to inform userspace that the configuration was not
applied instead of silently ignoring the error.
> } else {
> /* RS232 without hadware handshake or controlled by GPIOs */
> mode |= ATMEL_US_USMODE_NORMAL;
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 6/8] ARM: Change MPIDR_AFFINITY_LEVEL to ignore Aff3
From: Vladimir Murzin @ 2016-09-12 9:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D67A06.8090107@arm.com>
On 12/09/16 10:48, Marc Zyngier wrote:
> On 12/09/16 10:39, Vladimir Murzin wrote:
>> On 09/09/16 17:59, Marc Zyngier wrote:
>>> On 08/09/16 17:06, Vladimir Murzin wrote:
>>>> vgic-v3 driver queries CPU affinity level up to Aff3, which is valid
>>>> for arm64. However, for arm up to Aff2 levels are supported, so
>>>> querying for third level ends with upper bits of MPIDR are treated as
>>>> valid affinity level which is not true. Make sure we report zero for
>>>> any affinity level above two.
>>>>
>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>>> ---
>>>> arch/arm/include/asm/cputype.h | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>>>> index 1ee94c7..f08fac4 100644
>>>> --- a/arch/arm/include/asm/cputype.h
>>>> +++ b/arch/arm/include/asm/cputype.h
>>>> @@ -55,9 +55,10 @@
>>>>
>>>> #define MPIDR_LEVEL_BITS 8
>>>> #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
>>>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>>>>
>>>> #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>>>> - ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
>>>> + (((mpidr & MPIDR_HWID_BITMASK) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>>>>
>>>> #define ARM_CPU_IMP_ARM 0x41
>>>> #define ARM_CPU_IMP_INTEL 0x69
>>>>
>>>
>>> There is something I don't quite get. Is this patch really necessary?
>>> Are there cases where we construct a MPIDR that can have Aff3 set on 32bit?
>>
>> I've just checked and it seems that I've been keeping the change for
>> MPIDR_AFFINITY_LEVEL() since vgic-old era where it was used with
>>
>> static u32 compress_mpidr(unsigned long mpidr)
>> {
>> u32 ret;
>>
>> ret = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8;
>> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16;
>> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24;
>>
>> return ret;
>> }
>>
>> and now that code gone and nobody passes level 3 into this macro, so I
>> can drop this change.
>>
>> However, I do need MPIDR_LEVEL_SHIFT() macro since it is used in vgic-v3.
>>
>> Should I leave patch as is or you have something in mind?
>
> I think that for the sake of keeping the change minimal, it'd be better
> to just add the MPIDR_LEVEL_SHIFT macro, and drop the other changes.
Noted!
Thanks
Vladimir
>
> Thanks,
>
> M.
>
^ permalink raw reply
* [PATCH v3 6/8] ARM: Change MPIDR_AFFINITY_LEVEL to ignore Aff3
From: Marc Zyngier @ 2016-09-12 9:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D677EE.4080706@arm.com>
On 12/09/16 10:39, Vladimir Murzin wrote:
> On 09/09/16 17:59, Marc Zyngier wrote:
>> On 08/09/16 17:06, Vladimir Murzin wrote:
>>> vgic-v3 driver queries CPU affinity level up to Aff3, which is valid
>>> for arm64. However, for arm up to Aff2 levels are supported, so
>>> querying for third level ends with upper bits of MPIDR are treated as
>>> valid affinity level which is not true. Make sure we report zero for
>>> any affinity level above two.
>>>
>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>>> ---
>>> arch/arm/include/asm/cputype.h | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>>> index 1ee94c7..f08fac4 100644
>>> --- a/arch/arm/include/asm/cputype.h
>>> +++ b/arch/arm/include/asm/cputype.h
>>> @@ -55,9 +55,10 @@
>>>
>>> #define MPIDR_LEVEL_BITS 8
>>> #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
>>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>>>
>>> #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>>> - ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
>>> + (((mpidr & MPIDR_HWID_BITMASK) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>>>
>>> #define ARM_CPU_IMP_ARM 0x41
>>> #define ARM_CPU_IMP_INTEL 0x69
>>>
>>
>> There is something I don't quite get. Is this patch really necessary?
>> Are there cases where we construct a MPIDR that can have Aff3 set on 32bit?
>
> I've just checked and it seems that I've been keeping the change for
> MPIDR_AFFINITY_LEVEL() since vgic-old era where it was used with
>
> static u32 compress_mpidr(unsigned long mpidr)
> {
> u32 ret;
>
> ret = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8;
> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16;
> ret |= MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24;
>
> return ret;
> }
>
> and now that code gone and nobody passes level 3 into this macro, so I
> can drop this change.
>
> However, I do need MPIDR_LEVEL_SHIFT() macro since it is used in vgic-v3.
>
> Should I leave patch as is or you have something in mind?
I think that for the sake of keeping the change minimal, it'd be better
to just add the MPIDR_LEVEL_SHIFT macro, and drop the other changes.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v4 4/8] dts: sun8i-h3: move uart1 pinmux/peripheral assocation to DSTI
From: Maxime Ripard @ 2016-09-12 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160908095108.GA14915@carbon.kippendief.biz>
On Thu, Sep 08, 2016 at 11:51:09AM +0200, Jorik Jonker wrote:
> >>- put rts/cts in seperate pinmux sets for uart1 (2,3: see below)
> >>- associate rx/tx for uart1-3 in H3 DTSI (this is the only option)
> >
> >I'm still a bit skeptical about this. This wouldn't be in any way
> >consistant. I prefer to have something consistant and a bit duplicated
> >over something without any duplication but that confuses everyone
> >about what should be placed where.
> >
> >>- associate UART1 rts/cts as pinctrl-1 in sun8i-h3-bananapi-m2-plus
> >> (to prevent breakage for existing users)
> >
> >You can also set it in pinctrl-0.
>
> OK, sounds reasonable, but also a bit contradictive. One the one hand you
> prefer consistency (so, let uart2-3 follow uart1 and include rts/cts in
> them)
Hmm, I never said that, quite the opposite actually. Any board might
use either just RX/TX, or RX/TX and RTS/CTS. I don't see why we should
enable RTS/CTS on any board by default.
> , on the other hand the common case over the rare (so split off
> rts/cts). What should I do with uarts2-3 and should I do that to
> uart1 too?
You do the exact same thing in both cases.
My point was that you could just do:
pinctrl-0 = <&uart0_pins_a>, <&uart0_rts_cts_pins_a>;
pinctrl-names = "default";
instead of
pinctrl-0 = <&uart0_pins_a>;
pinctrl-1 = <&uart0_rts_cts_pins_a>;
pinctrl-names = "default", "default";
Since they are the exact same pin state.
> Moreover, Chen-Yu prefers to drop _a and @0 when they are redundant, which
> does not appear to be the convention, looking at existing sun*dsti. What's
> your opinion on this?
AFAIK, he wanted to remove them when they're not relevant (ie, only
one pin state possible).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160912/2eaa8fe0/attachment.sig>
^ permalink raw reply
* [PATCHv2 3/3] tty/serial: at91: fix hardware handshake on SAM9x5 (without GPIOs)
From: Richard Genoud @ 2016-09-12 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912094733.21501-1-richard.genoud@gmail.com>
Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake on SAM9x5
platforms.
On Atmel platforms, the USART can only handle the handware handshake
(ATMEL_US_USMODE_HWHS) if FIFOs or PDC are used.
Thus, ATMEL_US_USMODE_HWHS mode should only be used in this case.
For SAM9x5, there's no FIFOs nor PDC for the USART, so the mode should
be ATMEL_US_USMODE_NORMAL and the RTS pin should be controlled by the
driver.
NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
Tested on SAM9G35-CM with and without DMA
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
drivers/tty/serial/atmel_serial.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index e9b4fbf88c2d..32154e7231ce 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2130,15 +2130,19 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
} else if ((termios->c_cflag & CRTSCTS) &&
!mctrl_gpio_use_rtscts(atmel_port->gpios)) {
/*
- * RS232 with hardware handshake (RTS/CTS)
- * handled by the controller.
+ * Automatic hardware handshake (RTS/CTS) only work with
+ * FIFOs or PDC.
+ * Meaning that on SAM9x5 the controller can't handle
+ * the hardware handshake (no FIFOs nor PDC on these platforms).
*/
- if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
- dev_info(port->dev, "not enabling hardware flow control because DMA is used");
- termios->c_cflag &= ~CRTSCTS;
- } else {
+ if (atmel_use_pdc_rx(port) || atmel_use_fifo(port))
mode |= ATMEL_US_USMODE_HWHS;
- }
+ else
+ /*
+ * The hardware handshake won't be handle by the
+ * controller but by the driver.
+ */
+ mode |= ATMEL_US_USMODE_NORMAL;
} else {
/* RS232 without hadware handshake or controlled by GPIOs */
mode |= ATMEL_US_USMODE_NORMAL;
^ permalink raw reply related
* [PATCHv2 2/3] tty/serial: at91: fix hardware handshake with GPIOs
From: Richard Genoud @ 2016-09-12 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912094733.21501-1-richard.genoud@gmail.com>
Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled") broke the hardware handshake when GPIOs
where used.
Hardware handshake with GPIOs used to work before this commit because
the CRTSCTS flag (termios->c_cflag) was set, but not the
ATMEL_US_USMODE_HWHS flag (controller register) ; so hardware handshake
enabled, but not handled by the controller.
This commit restores this behaviour.
NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
and it will also need previous commit:
"serial: mctrl_gpio: implement mctrl_gpio_use_rtscts"
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
---
drivers/tty/serial/atmel_serial.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 2eaa18ddef61..e9b4fbf88c2d 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -2025,6 +2025,7 @@ static void atmel_serial_pm(struct uart_port *port, unsigned int state,
static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
struct ktermios *old)
{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
unsigned long flags;
unsigned int old_mode, mode, imr, quot, baud;
@@ -2126,8 +2127,12 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
atmel_uart_writel(port, ATMEL_US_TTGR,
port->rs485.delay_rts_after_send);
mode |= ATMEL_US_USMODE_RS485;
- } else if (termios->c_cflag & CRTSCTS) {
- /* RS232 with hardware handshake (RTS/CTS) */
+ } else if ((termios->c_cflag & CRTSCTS) &&
+ !mctrl_gpio_use_rtscts(atmel_port->gpios)) {
+ /*
+ * RS232 with hardware handshake (RTS/CTS)
+ * handled by the controller.
+ */
if (atmel_use_dma_rx(port) && !atmel_use_fifo(port)) {
dev_info(port->dev, "not enabling hardware flow control because DMA is used");
termios->c_cflag &= ~CRTSCTS;
@@ -2135,7 +2140,7 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
mode |= ATMEL_US_USMODE_HWHS;
}
} else {
- /* RS232 without hadware handshake */
+ /* RS232 without hadware handshake or controlled by GPIOs */
mode |= ATMEL_US_USMODE_NORMAL;
}
^ permalink raw reply related
* [PATCHv2 1/3] serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
From: Richard Genoud @ 2016-09-12 9:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912094733.21501-1-richard.genoud@gmail.com>
This function returns true if CTS and RTS are used as GPIOs.
Some drivers (like atmel_serial) needs to know if the flow control is
handled by the controller or by GPIOs.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
drivers/tty/serial/serial_mctrl_gpio.c | 8 ++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/tty/serial/serial_mctrl_gpio.c b/drivers/tty/serial/serial_mctrl_gpio.c
index d2da6aa7f27d..93bed8c99796 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.c
+++ b/drivers/tty/serial/serial_mctrl_gpio.c
@@ -17,6 +17,7 @@
#include <linux/err.h>
#include <linux/device.h>
#include <linux/irq.h>
+#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/termios.h>
#include <linux/serial_core.h>
@@ -72,6 +73,13 @@ struct gpio_desc *mctrl_gpio_to_gpiod(struct mctrl_gpios *gpios,
}
EXPORT_SYMBOL_GPL(mctrl_gpio_to_gpiod);
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+ return !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)) &&
+ !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_RTS));
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_use_rtscts);
+
unsigned int mctrl_gpio_get(struct mctrl_gpios *gpios, unsigned int *mctrl)
{
enum mctrl_gpio_idx i;
diff --git a/drivers/tty/serial/serial_mctrl_gpio.h b/drivers/tty/serial/serial_mctrl_gpio.h
index fa000bcff217..c34269733c62 100644
--- a/drivers/tty/serial/serial_mctrl_gpio.h
+++ b/drivers/tty/serial/serial_mctrl_gpio.h
@@ -101,6 +101,11 @@ void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
*/
void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
+/*
+ * Return true if both CTS and RTS are used with GPIOs
+ */
+bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios);
+
#else /* GPIOLIB */
static inline
@@ -152,6 +157,11 @@ static inline void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
{
}
+static inline bool mctrl_gpio_use_rtscts(struct mctrl_gpios *gpios)
+{
+ return false;
+}
+
#endif /* GPIOLIB */
#endif
^ permalink raw reply related
* [PATCHv2 0/3] Fix handware handshake on SAM9x5 platforms
From: Richard Genoud @ 2016-09-12 9:47 UTC (permalink / raw)
To: linux-arm-kernel
Since commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
hardware handshake is enabled"), hardware handshake is not working
anymore on SAM9x5 platforms.
The first two patches fix the hardware handshake when CTS/RTS pins are
handle by GPIOs.
The last patch fixes hardware handshake when CTS/RTS pins are not GPIOs.
Changes since v1:
- Correct patch 1 with the error found by kbuild.
- Add Alexandre's Acked-by on patch 2
- Rewrite patch 3 logic in the light of the on-going discussion
with Cyrille and Alexandre.
NB: patch 2 NEEDS patch 1 to compile.
Richard Genoud (3):
serial: mctrl_gpio: implement mctrl_gpio_use_rtscts
tty/serial: at91: fix hardware handshake with GPIOs
tty/serial: at91: fix hardware handshake when DMA is not used
drivers/tty/serial/atmel_serial.c | 25 +++++++++++++++++--------
drivers/tty/serial/serial_mctrl_gpio.c | 8 ++++++++
drivers/tty/serial/serial_mctrl_gpio.h | 10 ++++++++++
3 files changed, 35 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH v15.1 3/5] drm/rockchip: cdn-dp: add cdn DP support for rk3399
From: Chris Zhong @ 2016-09-12 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912091339.GY27946@sirena.org.uk>
Hi Mark
OK, thanks.
I will send the whole series next time, hope it will not bother anyone
On 09/12/2016 05:13 PM, Mark Brown wrote:
> On Fri, Sep 09, 2016 at 09:16:06PM -0700, Chris Zhong wrote:
>> Add support for cdn DP controller which is embedded in the rk3399
>> SoCs. The DP is compliant with DisplayPort Specification,
> Please don't new patches in reply to old serieses, especially not
> individual patches in the middle of the series - it just makes
> everything more confusing. It becomes difficult to tell what the
> version of the series that's actually expected is. Please send the
> whole series.
^ permalink raw reply
* [PATCH v3 7/8] ARM: Move system register accessors to asm/cp15.h
From: Vladimir Murzin @ 2016-09-12 9:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2EBE4.4060107@arm.com>
Sorry, missed this part...
On 09/09/16 18:05, Marc Zyngier wrote:
>>
>> > +#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
>> > + "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
>> > +#define __ACCESS_CP15_64(Op1, CRm) \
>> > + "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
>> > +
>> > +#define __read_sysreg(r, w, c, t) ({ \
>> > + t __val; \
>> > + asm volatile(r " " c : "=r" (__val)); \
>> > + __val; \
>> > +})
>> > +#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
>> > +
>> > +#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
>> > +#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
>> > +
> Shouldn't that be placed after the #ifdef below?
>
Yes, I'll move it under #ifdef.
Thanks
Vladimir
^ permalink raw reply
* [PATCH v5 2/3] mfd: add support for Allwinner SoCs ADC
From: Quentin Schulz @ 2016-09-12 9:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912091829.GB1873@dell>
On 12/09/2016 11:18, Lee Jones wrote:
> On Thu, 08 Sep 2016, Quentin Schulz wrote:
>
[...]
>> + To compile this driver as a module, choose M here: the module will be
>> + called sun4i-gpadc-mfd.
>
> Drop the -mfd.
>
>> config MFD_AS3711
>> bool "AMS AS3711"
>> select MFD_CORE
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 42a66e1..3b964d7 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -205,3 +205,5 @@ intel-soc-pmic-objs := intel_soc_pmic_core.o intel_soc_pmic_crc.o
>> intel-soc-pmic-$(CONFIG_INTEL_PMC_IPC) += intel_soc_pmic_bxtwc.o
>> obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
>> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>> +
>> +obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc-mfd.o
>> diff --git a/drivers/mfd/sun4i-gpadc-mfd.c b/drivers/mfd/sun4i-gpadc-mfd.c
>> new file mode 100644
>> index 0000000..b499545
>> --- /dev/null
>> +++ b/drivers/mfd/sun4i-gpadc-mfd.c
>
> Drop the -mfd.
>
>> @@ -0,0 +1,174 @@
>> +/* ADC MFD core driver for sunxi platforms
>> + *
>> + * Copyright (c) 2016 Quentin Schulz <quentin.schulz@free-electrons.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + */
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/mfd/sun4i-gpadc-mfd.h>
>> +
>> +static struct resource adc_resources[] = {
>> + {
>> + .name = "FIFO_DATA_PENDING",
>> + .start = SUN4I_GPADC_IRQ_FIFO_DATA,
>> + .end = SUN4I_GPADC_IRQ_FIFO_DATA,
>> + .flags = IORESOURCE_IRQ,
>> + }, {
>> + .name = "TEMP_DATA_PENDING",
>> + .start = SUN4I_GPADC_IRQ_TEMP_DATA,
>> + .end = SUN4I_GPADC_IRQ_TEMP_DATA,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> +};
>
> Use the RES_IRQ_* defines.
>
>> +static const struct regmap_irq sun4i_gpadc_mfd_regmap_irq[] = {
>> + REGMAP_IRQ_REG(SUN4I_GPADC_IRQ_FIFO_DATA, 0,
>> + SUN4I_GPADC_INT_FIFOC_TP_DATA_IRQ_EN),
>> + REGMAP_IRQ_REG(SUN4I_GPADC_IRQ_TEMP_DATA, 0,
>> + SUN4I_GPADC_INT_FIFOC_TEMP_IRQ_EN),
>> +};
>> +
>> +static const struct regmap_irq_chip sun4i_gpadc_mfd_regmap_irq_chip = {
>> + .name = "sun4i_gpadc_mfd_irq_chip",
>> + .status_base = SUN4I_GPADC_INT_FIFOS,
>> + .ack_base = SUN4I_GPADC_INT_FIFOS,
>> + .mask_base = SUN4I_GPADC_INT_FIFOC,
>> + .init_ack_masked = true,
>> + .mask_invert = true,
>> + .irqs = sun4i_gpadc_mfd_regmap_irq,
>> + .num_irqs = ARRAY_SIZE(sun4i_gpadc_mfd_regmap_irq),
>> + .num_regs = 1,
>> +};
>> +
>> +static struct mfd_cell sun4i_gpadc_mfd_cells[] = {
>> + {
>> + .name = "sun4i-a10-gpadc-iio",
>> + .resources = adc_resources,
>> + .num_resources = ARRAY_SIZE(adc_resources),
>> + }, {
>> + .name = "iio_hwmon",
>> + }
>
> Single line please
>
> { .name = "iio_hwmon" }
>
+ {
+ .name = "sun4i-a10-gpadc-iio",
+ .resources = adc_resources,
+ .num_resources = ARRAY_SIZE(adc_resources),
+ }, { .name = "iio_hwmon" }
or
+ {
+ .name = "sun4i-a10-gpadc-iio",
+ .resources = adc_resources,
+ .num_resources = ARRAY_SIZE(adc_resources),
+ },
+ { .name = "iio_hwmon" }
?
>> +};
>> +
>> +static struct mfd_cell sun5i_gpadc_mfd_cells[] = {
>> + {
>> + .name = "sun5i-a13-gpadc-iio",
>> + .resources = adc_resources,
>> + .num_resources = ARRAY_SIZE(adc_resources),
>> + }, {
>> + .name = "iio_hwmon",
>> + },
>> +};
>
> As above.
>
>> +static struct mfd_cell sun6i_gpadc_mfd_cells[] = {
>> + {
>> + .name = "sun6i-a31-gpadc-iio",
>> + .resources = adc_resources,
>> + .num_resources = ARRAY_SIZE(adc_resources),
>> + }, {
>> + .name = "iio_hwmon",
>> + },
>> +};
>
> As above.
>
>> +static const struct regmap_config sun4i_gpadc_mfd_regmap_config = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .fast_io = true,
>> +};
>> +
>> +static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
>> + {
>> + .compatible = "allwinner,sun4i-a10-ts",
>> + .data = &sun4i_gpadc_mfd_cells,
>> + }, {
>> + .compatible = "allwinner,sun5i-a13-ts",
>> + .data = &sun5i_gpadc_mfd_cells,
>> + }, {
>> + .compatible = "allwinner,sun6i-a31-ts",
>> + .data = &sun6i_gpadc_mfd_cells,
>> + }, { /* sentinel */ }
>> +};
>
> Don't mix OF and MFD functionality.
>
> Why don't you create a node for "iio_hwmon" and have
> platform_of_populate() do your bidding?
>
We are using a stable binding which we cannot modify. This means, the DT
in its current state can only be modified to add features, which is not
the case of this driver (it is a rewriting of an existing driver which
uses the rtp node).
>> +static int sun4i_gpadc_mfd_probe(struct platform_device *pdev)
>
> Remove all mention of "mfd" from this file.
>
> (Accept the calls to the MFD API of course).
>
[...]
>> +
>> +MODULE_DEVICE_TABLE(of, sun4i_gpadc_mfd_of_match);
>
> Place this directly under the table.
>
>> +static struct platform_driver sun4i_gpadc_mfd_driver = {
>> + .driver = {
>> + .name = "sun4i-adc-mfd",
>> + .of_match_table = of_match_ptr(sun4i_gpadc_mfd_of_match),
>> + },
>> + .probe = sun4i_gpadc_mfd_probe,
>
> No .remove?
>
No, everything in probe is handled with devm functions.
[...]
>> +struct sun4i_gpadc_mfd_dev {
>> + struct device *dev;
>> + struct regmap *regmap;
>> + struct regmap_irq_chip_data *regmap_irqc;
>> + void __iomem *regs;
>
> It's *much* more common to call this 'base'.
>
>> +};
>> +
>> +#endif
>
ACK for everything else.
Thanks,
Quentin
^ permalink raw reply
* [PATCH v3 7/8] ARM: Move system register accessors to asm/cp15.h
From: Vladimir Murzin @ 2016-09-12 9:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2EBE4.4060107@arm.com>
On 09/09/16 18:05, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> Headers linux/irqchip/arm-gic.v3.h and arch/arm/include/asm/kvm_hyp.h
>> are included in virt/kvm/arm/hyp/vgic-v3-sr.c and both define macros
>> called __ACCESS_CP15 and __ACCESS_CP15_64 which obviously creates a
>> conflict. These macros were introduced independently for GIC and KVM
>> and, in fact, do the same thing.
>>
>> As an option we could add prefixes to KVM and GIC version of macros so
>> they won't clash, but it'd introduce code duplication. Alternatively,
>> we could keep macro in, say, GIC header and include it in KVM one (or
>> vice versa), but such dependency would not look nicer.
>>
>> So we follow arm64 way (it handles this via sysreg.h) and move only
>> single set of macros to asm/cp15.h
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> arch/arm/include/asm/arch_gicv3.h | 27 +++++++++++----------------
>> arch/arm/include/asm/cp15.h | 15 +++++++++++++++
>> arch/arm/include/asm/kvm_hyp.h | 15 +--------------
>> 3 files changed, 27 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
>> index e08d151..af25c32 100644
>> --- a/arch/arm/include/asm/arch_gicv3.h
>> +++ b/arch/arm/include/asm/arch_gicv3.h
>> @@ -22,9 +22,7 @@
>>
>> #include <linux/io.h>
>> #include <asm/barrier.h>
>> -
>> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2) p15, Op1, %0, CRn, CRm, Op2
>> -#define __ACCESS_CP15_64(Op1, CRm) p15, Op1, %Q0, %R0, CRm
>> +#include <asm/cp15.h>
>>
>> #define ICC_EOIR1 __ACCESS_CP15(c12, 0, c12, 1)
>> #define ICC_DIR __ACCESS_CP15(c12, 0, c11, 1)
>> @@ -102,58 +100,55 @@
>>
>> static inline void gic_write_eoir(u32 irq)
>> {
>> - asm volatile("mcr " __stringify(ICC_EOIR1) : : "r" (irq));
>> + write_sysreg(irq, ICC_EOIR1);
>> isb();
>> }
>>
>> static inline void gic_write_dir(u32 val)
>> {
>> - asm volatile("mcr " __stringify(ICC_DIR) : : "r" (val));
>> + write_sysreg(val, ICC_DIR);
>> isb();
>> }
>>
>> static inline u32 gic_read_iar(void)
>> {
>> - u32 irqstat;
>> + u32 irqstat = read_sysreg(ICC_IAR1);
>>
>> - asm volatile("mrc " __stringify(ICC_IAR1) : "=r" (irqstat));
>> dsb(sy);
>> +
>> return irqstat;
>> }
>>
>> static inline void gic_write_pmr(u32 val)
>> {
>> - asm volatile("mcr " __stringify(ICC_PMR) : : "r" (val));
>> + write_sysreg(val, ICC_PMR);
>> }
>>
>> static inline void gic_write_ctlr(u32 val)
>> {
>> - asm volatile("mcr " __stringify(ICC_CTLR) : : "r" (val));
>> + write_sysreg(val, ICC_CTLR);
>> isb();
>> }
>>
>> static inline void gic_write_grpen1(u32 val)
>> {
>> - asm volatile("mcr " __stringify(ICC_IGRPEN1) : : "r" (val));
>> + write_sysreg(val, ICC_IGRPEN1);
>> isb();
>> }
>>
>> static inline void gic_write_sgi1r(u64 val)
>> {
>> - asm volatile("mcrr " __stringify(ICC_SGI1R) : : "r" (val));
>> + write_sysreg(val, ICC_SGI1R);
>> }
>>
>> static inline u32 gic_read_sre(void)
>> {
>> - u32 val;
>> -
>> - asm volatile("mrc " __stringify(ICC_SRE) : "=r" (val));
>> - return val;
>> + return read_sysreg(ICC_SRE);
>> }
>>
>> static inline void gic_write_sre(u32 val)
>> {
>> - asm volatile("mcr " __stringify(ICC_SRE) : : "r" (val));
>> + write_sysreg(val, ICC_SRE);
>> isb();
>> }
>>
>> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
>> index c3f1152..f661732 100644
>> --- a/arch/arm/include/asm/cp15.h
>> +++ b/arch/arm/include/asm/cp15.h
>> @@ -47,6 +47,21 @@
>> #define vectors_high() (0)
>> #endif
>>
>> +#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
>> + "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
>> +#define __ACCESS_CP15_64(Op1, CRm) \
>> + "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
>> +
>> +#define __read_sysreg(r, w, c, t) ({ \
>> + t __val; \
>> + asm volatile(r " " c : "=r" (__val)); \
>> + __val; \
>> +})
>> +#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
>> +
>> +#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
>> +#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
>> +
>
> Shouldn't that be placed after the #ifdef below?
>
>> #ifdef CONFIG_CPU_CP15
>>
>> extern unsigned long cr_alignment; /* defined in entry-armv.S */
>> diff --git a/arch/arm/include/asm/kvm_hyp.h b/arch/arm/include/asm/kvm_hyp.h
>> index bd9434e..0b475d2 100644
>> --- a/arch/arm/include/asm/kvm_hyp.h
>> +++ b/arch/arm/include/asm/kvm_hyp.h
>> @@ -20,26 +20,13 @@
>>
>> #include <linux/compiler.h>
>> #include <linux/kvm_host.h>
>> +#include <asm/cp15.h>
>> #include <asm/kvm_mmu.h>
>> #include <asm/vfp.h>
>>
>> -#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
>> - "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
>> -#define __ACCESS_CP15_64(Op1, CRm) \
>> - "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
>> #define __ACCESS_VFP(CRn) \
>> "mrc", "mcr", __stringify(p10, 7, %0, CRn, cr0, 0), u32
>>
>> -#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
>> -#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
>> -
>> -#define __read_sysreg(r, w, c, t) ({ \
>> - t __val; \
>> - asm volatile(r " " c : "=r" (__val)); \
>> - __val; \
>> -})
>> -#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
>> -
>> #define write_special(v, r) \
>> asm volatile("msr " __stringify(r) ", %0" : : "r" (v))
>> #define read_special(r) ({ \
>>
>
> Could you please cc RMK on this, given that this touches a core arch/arm
> file?
Ok. I'll cc/to him with the next re-spin.
Cheers
Vladimir
>
> Thanks,
>
> M.
>
^ permalink raw reply
* [PATCH v3 6/8] ARM: Change MPIDR_AFFINITY_LEVEL to ignore Aff3
From: Vladimir Murzin @ 2016-09-12 9:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2EA7F.9030705@arm.com>
On 09/09/16 17:59, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> vgic-v3 driver queries CPU affinity level up to Aff3, which is valid
>> for arm64. However, for arm up to Aff2 levels are supported, so
>> querying for third level ends with upper bits of MPIDR are treated as
>> valid affinity level which is not true. Make sure we report zero for
>> any affinity level above two.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> arch/arm/include/asm/cputype.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
>> index 1ee94c7..f08fac4 100644
>> --- a/arch/arm/include/asm/cputype.h
>> +++ b/arch/arm/include/asm/cputype.h
>> @@ -55,9 +55,10 @@
>>
>> #define MPIDR_LEVEL_BITS 8
>> #define MPIDR_LEVEL_MASK ((1 << MPIDR_LEVEL_BITS) - 1)
>> +#define MPIDR_LEVEL_SHIFT(level) (MPIDR_LEVEL_BITS * level)
>>
>> #define MPIDR_AFFINITY_LEVEL(mpidr, level) \
>> - ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK)
>> + (((mpidr & MPIDR_HWID_BITMASK) >> MPIDR_LEVEL_SHIFT(level)) & MPIDR_LEVEL_MASK)
>>
>> #define ARM_CPU_IMP_ARM 0x41
>> #define ARM_CPU_IMP_INTEL 0x69
>>
>
> There is something I don't quite get. Is this patch really necessary?
> Are there cases where we construct a MPIDR that can have Aff3 set on 32bit?
I've just checked and it seems that I've been keeping the change for
MPIDR_AFFINITY_LEVEL() since vgic-old era where it was used with
static u32 compress_mpidr(unsigned long mpidr)
{
u32 ret;
ret = MPIDR_AFFINITY_LEVEL(mpidr, 0);
ret |= MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8;
ret |= MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16;
ret |= MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24;
return ret;
}
and now that code gone and nobody passes level 3 into this macro, so I
can drop this change.
However, I do need MPIDR_LEVEL_SHIFT() macro since it is used in vgic-v3.
Should I leave patch as is or you have something in mind?
Cheers
Vladimir
>
> Thanks,
>
> M.
>
^ permalink raw reply
* [GIT PULL 1/4] ARM: exynos: SoC/Mach for v4.9
From: Sylwester Nawrocki @ 2016-09-12 9:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160908192434.GA609@sam-MacBookPro>
On 09/08/2016 09:24 PM, Sam Van Den Berge wrote:
> Thanks for the suggestion and apologies for the late response. Last couple of
> days I've been figuring out what this would involve and I decided that I want
> to give this a try. I found this patch series [1] that I'll use as an example.
> If I understand it correctly, I'll need to create a dma_slave_map and
> pass that via the platform data into the s3c24xx-dma driver. There I'll
> need to fill in the filter_map of the dma slave device. Right?
Yes, AFAIU that's what needs to be done. Subsequently, related DMA
clients could be updated to not use dma_request_slave_channel_compat().
--
Thanks,
Sylwester
^ permalink raw reply
* [kernel-hardening] Re: [PATCH v2 3/7] arm64: Introduce uaccess_{disable, enable} functionality based on TTBR0_EL1
From: Catalin Marinas @ 2016-09-12 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu8GMU=Lgh4awFLda-7K=orpg03D18kPDVVQEP6KzB5++g@mail.gmail.com>
On Sun, Sep 11, 2016 at 02:55:12PM +0100, Ard Biesheuvel wrote:
> On 6 September 2016 at 11:45, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Tue, Sep 06, 2016 at 11:27:42AM +0100, Catalin Marinas wrote:
> >> On Mon, Sep 05, 2016 at 06:20:38PM +0100, Mark Rutland wrote:
> >> > On Fri, Sep 02, 2016 at 04:02:09PM +0100, Catalin Marinas wrote:
> >> > > * tables again to remove any speculatively loaded cache lines.
> >> > > */
> >> > > mov x0, x25
> >> > > - add x1, x26, #SWAPPER_DIR_SIZE
> >> > > + add x1, x26, #SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE
> >> > > dmb sy
> >> > > bl __inval_cache_range
> >> > >
> >> > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> >> > > index 659963d40bb4..fe393ccf9352 100644
> >> > > --- a/arch/arm64/kernel/vmlinux.lds.S
> >> > > +++ b/arch/arm64/kernel/vmlinux.lds.S
> >> > > @@ -196,6 +196,11 @@ SECTIONS
> >> > > swapper_pg_dir = .;
> >> > > . += SWAPPER_DIR_SIZE;
> >> > >
> >> > > +#ifdef CONFIG_ARM64_TTBR0_PAN
> >> > > + reserved_ttbr0 = .;
> >> > > + . += PAGE_SIZE;
> >> > > +#endif
> >> >
> >> > Surely RESERVED_TTBR0_SIZE, as elsewhere?
> >>
> >> I'll try to move it somewhere where it can be included in vmlinux.lds.S
> >> (I can probably include cpufeature.h directly).
>
> Do we really need another zero page? The ordinary zero page is already
> statically allocated these days, so we could simply move it between
> idmap_pg_dir[] and swapper_pg_dir[], and get all the changes in the
> early boot code for free (given that it covers the range between the
> start of idmap_pg_dir[] and the end of swapper_pg_dir[])
>
> That way, we could refer to __pa(empty_zero_page) anywhere by reading
> ttbr1_el1 and subtracting PAGE_SIZE
That's fine by me. I'll cherry-pick your patch and rebase this series on
top.
--
Catalin
^ permalink raw reply
* [PATCH v3 5/8] KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host systems
From: Vladimir Murzin @ 2016-09-12 9:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2E96B.70909@arm.com>
On 09/09/16 17:55, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> We have couple of 64-bit registers defined in GICv3 architecture, so
>> unsigned long accesses to these registers will only access a single
>> 32-bit part of that regitser. On the other hand these registers can't
>> be accessed as 64-bit with a single instruction like ldrd/strd or
>> ldmia/stmia if we run a 32-bit host because KVM does not support
>> access to MMIO space done by these instructions.
>>
>> It means that a 32-bit guest accesses these registers in 32-bit
>> chunks, so the only thing we need to do is to ensure that
>> extract_bytes() always takes 64-bit data.
>>
>> Since we are here fix couple of other width related issues catched by
>> gcc
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> virt/kvm/arm/vgic/vgic-mmio-v3.c | 6 +++---
>> virt/kvm/arm/vgic/vgic-mmio.h | 2 +-
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> index acbe691..0d3c76a 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
>> @@ -23,7 +23,7 @@
>> #include "vgic-mmio.h"
>>
>> /* extract @num bytes at @offset bytes offset in data */
>> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
>> +unsigned long extract_bytes(u64 data, unsigned int offset,
>> unsigned int num)
>> {
>> return (data >> (offset * 8)) & GENMASK_ULL(num * 8 - 1, 0);
>> @@ -181,7 +181,7 @@ static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
>> int target_vcpu_id = vcpu->vcpu_id;
>> u64 value;
>>
>> - value = (mpidr & GENMASK(23, 0)) << 32;
>> + value = (u64)(mpidr & GENMASK(23, 0)) << 32;
>> value |= ((target_vcpu_id & 0xffff) << 8);
>> if (target_vcpu_id == atomic_read(&vcpu->kvm->online_vcpus) - 1)
>> value |= GICR_TYPER_LAST;
>> @@ -611,7 +611,7 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
>> bool broadcast;
>>
>> sgi = (reg & ICC_SGI1R_SGI_ID_MASK) >> ICC_SGI1R_SGI_ID_SHIFT;
>> - broadcast = reg & BIT(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
>> + broadcast = reg & BIT_ULL(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
>> target_cpus = (reg & ICC_SGI1R_TARGET_LIST_MASK) >> ICC_SGI1R_TARGET_LIST_SHIFT;
>> mpidr = SGI_AFFINITY_LEVEL(reg, 3);
>> mpidr |= SGI_AFFINITY_LEVEL(reg, 2);
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
>> index 0b3ecf9..80f92ce 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.h
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
>> @@ -96,7 +96,7 @@ unsigned long vgic_data_mmio_bus_to_host(const void *val, unsigned int len);
>> void vgic_data_host_to_mmio_bus(void *buf, unsigned int len,
>> unsigned long data);
>>
>> -unsigned long extract_bytes(unsigned long data, unsigned int offset,
>> +unsigned long extract_bytes(u64 data, unsigned int offset,
>> unsigned int num);
>>
>> u64 update_64bit_reg(u64 reg, unsigned int offset, unsigned int len,
>>
>
> My personal preference would be to split this in two patches. One that
> changes extract_bytes to work on 64bit quantities, and another one that
> addresses the 64bit issues. Not a big deal though.
>
Anyway I have to resend, so I'll split it per your preference ;)
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>
Thanks!
Vladimir
> Thanks,
>
> M.
>
^ permalink raw reply
* [PATCH v3 4/8] KVM: arm64: vgic-its: Introduce config option to guard ITS specific code
From: Vladimir Murzin @ 2016-09-12 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2E76E.2000607@arm.com>
On 09/09/16 17:46, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> By now ITS code guarded with KVM_ARM_VGIC_V3 config option which was
>> introduced to hide everything specific to vgic-v3 from 32-bit world.
>> We are going to support vgic-v3 in 32-bit world and KVM_ARM_VGIC_V3
>> will gone, but we don't have support for ITS there yet and we need to
>> continue keeping ITS away.
>> Introduce the new config option to prevent ITS code being build in
>> 32-bit mode when support for vgic-v3 is done.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Thanks!
Vladimir
>
> M.
>
^ permalink raw reply
* [PATCH v3 8/8] ARM: KVM: Support vgic-v3
From: Vladimir Murzin @ 2016-09-12 9:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D2E746.7080300@arm.com>
On 09/09/16 17:45, Marc Zyngier wrote:
> On 08/09/16 17:06, Vladimir Murzin wrote:
>> This patch allows to build and use vgic-v3 in 32-bit mode.
>>
snip...
>> diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h
>> index af25c32..f93f6bd 100644
>> --- a/arch/arm/include/asm/arch_gicv3.h
>> +++ b/arch/arm/include/asm/arch_gicv3.h
>> @@ -96,6 +96,70 @@
>> #define ICH_AP1R2 __AP1Rx(2)
>> #define ICH_AP1R3 __AP1Rx(3)
>>
>> +/* A32-to-A64 mappings used by VGIC save/restore */
>> +
>> +#define CPUIF_MAP(a32, a64) \
>> +static inline void write_ ## a64(u32 val) \
>> +{ \
>> + write_sysreg(val, a32); \
>> +} \
>> +static inline u32 read_ ## a64(void) \
>> +{ \
>> + return read_sysreg(a32); \
>> +} \
>> +
>> +#define CPUIF_MAP_LO_HI(a32lo, a32hi, a64) \
>> +static inline void write_ ## a64(u64 val) \
>> +{ \
>> + write_sysreg((u32)val, a32lo); \
>> + write_sysreg((u32)(val >> 32), a32hi); \
>
> Please use {lower,upper}_32_bits, which make the casting/shifting go away.
>
Will do.
>> +} \
>> +static inline u64 read_ ## a64(void) \
>> +{ \
>> + u64 val = read_sysreg(a32lo); \
>> + \
>> + val |= (u64)read_sysreg(a32hi) << 32; \
>> + \
>> + return val; \
>> +}
>> +
>> +CPUIF_MAP(ICH_HCR, ICH_HCR_EL2)
>> +CPUIF_MAP(ICH_VTR, ICH_VTR_EL2)
>> +CPUIF_MAP(ICH_MISR, ICH_MISR_EL2)
>> +CPUIF_MAP(ICH_EISR, ICH_EISR_EL2)
>> +CPUIF_MAP(ICH_ELSR, ICH_ELSR_EL2)
>> +CPUIF_MAP(ICH_VMCR, ICH_VMCR_EL2)
>> +CPUIF_MAP(ICH_AP0R3, ICH_AP0R3_EL2)
>> +CPUIF_MAP(ICH_AP0R2, ICH_AP0R2_EL2)
>> +CPUIF_MAP(ICH_AP0R1, ICH_AP0R1_EL2)
>> +CPUIF_MAP(ICH_AP0R0, ICH_AP0R0_EL2)
>> +CPUIF_MAP(ICH_AP1R3, ICH_AP1R3_EL2)
>> +CPUIF_MAP(ICH_AP1R2, ICH_AP1R2_EL2)
>> +CPUIF_MAP(ICH_AP1R1, ICH_AP1R1_EL2)
>> +CPUIF_MAP(ICH_AP1R0, ICH_AP1R0_EL2)
>> +CPUIF_MAP(ICC_HSRE, ICC_SRE_EL2)
>> +CPUIF_MAP(ICC_SRE, ICC_SRE_EL1)
>> +
>> +CPUIF_MAP_LO_HI(ICH_LR15, ICH_LRC15, ICH_LR15_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR14, ICH_LRC14, ICH_LR14_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR13, ICH_LRC13, ICH_LR13_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR12, ICH_LRC12, ICH_LR12_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR11, ICH_LRC11, ICH_LR11_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR10, ICH_LRC10, ICH_LR10_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR9, ICH_LRC9, ICH_LR9_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR8, ICH_LRC8, ICH_LR8_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR7, ICH_LRC7, ICH_LR7_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR6, ICH_LRC6, ICH_LR6_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR5, ICH_LRC5, ICH_LR5_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR4, ICH_LRC4, ICH_LR4_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR3, ICH_LRC3, ICH_LR3_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR2, ICH_LRC2, ICH_LR2_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR1, ICH_LRC1, ICH_LR1_EL2)
>> +CPUIF_MAP_LO_HI(ICH_LR0, ICH_LRC0, ICH_LR0_EL2)
>> +
>> +#define read_gicreg(r) read_##r()
>> +#define write_gicreg(v, r) write_##r(v)
>> +
>
> Can you make this change a separate patch? It will make it easier to
> merge if I can ack it as a standalone change. It will also give the last
> patch a fantastic diffstat... ;-)
>
Yes, I can ;)
snip...
>> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
>> index 1bb2b79..10c0244 100644
>> --- a/arch/arm/kvm/coproc.c
>> +++ b/arch/arm/kvm/coproc.c
>> @@ -228,6 +228,36 @@ bool access_vm_reg(struct kvm_vcpu *vcpu,
>> return true;
>> }
>>
>> +static bool access_gic_sgi(struct kvm_vcpu *vcpu,
>> + const struct coproc_params *p,
>> + const struct coproc_reg *r)
>> +{
>> + u64 reg;
>> +
>> + if (!p->is_write)
>> + return read_from_write_only(vcpu, p);
>> +
>> + reg = *vcpu_reg(vcpu, p->Rt2);
>> + reg <<= 32;
>
> nit: can you write this as
>
> reg = (u64)*vcpu_reg(vcpu, p->Rt2) << 32;
>
> which I find easier to read...
>
I'll rewrite it.
snip...
>> diff --git a/arch/arm/kvm/hyp/Makefile b/arch/arm/kvm/hyp/Makefile
>> index 8dfa5f7..3023bb5 100644
>> --- a/arch/arm/kvm/hyp/Makefile
>> +++ b/arch/arm/kvm/hyp/Makefile
>> @@ -5,6 +5,7 @@
>> KVM=../../../../virt/kvm
>>
>> obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v2-sr.o
>> +obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/vgic-v3-sr.o
>> obj-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/hyp/timer-sr.o
>>
>> obj-$(CONFIG_KVM_ARM_HOST) += tlb.o
>> diff --git a/arch/arm/kvm/hyp/switch.c b/arch/arm/kvm/hyp/switch.c
>> index b13caa9..8409dd5 100644
>> --- a/arch/arm/kvm/hyp/switch.c
>> +++ b/arch/arm/kvm/hyp/switch.c
>> @@ -14,6 +14,7 @@
>
> It otherwise looks good to me.
>
Thanks for feedback!
Cheers
Vladimir
> Thanks,
>
> M.
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox