* Re: [PATCH v3 0/4] Add basic support for Mediatek MT8183 SoC
From: Matthias Brugger @ 2018-07-22 21:11 UTC (permalink / raw)
To: Erin Lo
Cc: Mark Rutland, devicetree, Jason Cooper, srv_heupstream,
Marc Zyngier, Greg Kroah-Hartman, linux-kernel, Rob Herring,
mars.cheng, linux-serial, yingjoe.chen, linux-mediatek,
Thomas Gleixner, linux-arm-kernel
In-Reply-To: <1532158134.16009.10.camel@mtksdaap41>
On 21/07/18 09:28, Erin Lo wrote:
> On Fri, 2018-07-20 at 12:44 +0200, Matthias Brugger wrote:
>>
>> On 20/07/18 10:19, Erin Lo wrote:
>>> On Mon, 2018-07-16 at 11:28 +0200, Matthias Brugger wrote:
>>>> Hi Erin,
>>>>
>>>> On 17/05/18 08:22, Erin Lo wrote:
>>>>> MT8183 is a SoC based on 64bit ARMv8 architecture.
>>>>> It contains 4 CA53 and 4 CA73 cores.
>>>>> MT8183 share many HW IP with MT65xx series.
>>>>> This patchset was tested on MT8183 evaluation board, and boot to shell ok.
>>>>>
>>>>> This series contains document bindings, device tree including interrupt, uart.
>>>>>
>>>>> Change in v3:
>>>>> 1. Fill out GICC, GICH, GICV regions
>>>>> 2. Update Copyright to 2018
>>>>>
>>>>> Change in v2:
>>>>> 1. Split dt-bindings into different patches
>>>>> 2. Correct bindings for supported SoCs (mtk-uart.txt)
>>>>>
>>>>> Ben Ho (1):
>>>>> arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
>>>>> Makefile
>>>>>
>>>>> Erin Lo (3):
>>>>> dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
>>>>> dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
>>>>> dt-bindings: serial: Add compatible for Mediatek MT8183
>>>>>
>>>>
>>>> I'm a bit reluctant to take this series, as it will only enable the EVB board to
>>>> boot into a serial console. Are you planning to add support for other devices of
>>>> this SoC?
>>>>
>>>> Apart please take into account that there is an issue with the dts file, as you
>>>> were told by the kbuild test robot.
>>>>
>>>> Regards,
>>>> Matthias
>>>>
>>>
>>> Hi, Matthias
>>> Sorry for missing this letter...since mail proxy server.
>>> We plan to add support all the devices of MT8183 in serious.
>>> We have implemented the clock and pinctrl driver for upstream and they
>>> are in internal review right now.
>>>
>>
>> Nice to hear that :)
>>
>>> About the dts issue... do you suggest me to send new patch right now or
>>> wait for clock and pinctrl driver ready then send them together?
>>>
>>
>> I would prefer that you send at least the clock controller together, so that we
>> don't have any dummy clocks in the basic device tree.
>>
>> Regards,
>> Matthias
>>
>
> Got it! Next patch, we will send them with clock controller without
> dummy clocks in the basic device tree.
> By the way, the clock controller driver of MT8183 is a little bit
> different from former ICs, so we need more time to prepare them.
> We will send them to public as soon as possible.
>
Thanks for the info.
Best regards,
Matthias
^ permalink raw reply
* Re: [PATCH v3 0/4] Add basic support for Mediatek MT8183 SoC
From: Erin Lo @ 2018-07-21 7:28 UTC (permalink / raw)
To: Matthias Brugger
Cc: Mark Rutland, devicetree, Jason Cooper, srv_heupstream,
Marc Zyngier, Greg Kroah-Hartman, linux-mediatek, linux-kernel,
Rob Herring, mars.cheng, linux-serial, yingjoe.chen,
Thomas Gleixner, linux-arm-kernel
In-Reply-To: <2ca76468-198b-b764-c974-374e567e0c93@gmail.com>
On Fri, 2018-07-20 at 12:44 +0200, Matthias Brugger wrote:
>
> On 20/07/18 10:19, Erin Lo wrote:
> > On Mon, 2018-07-16 at 11:28 +0200, Matthias Brugger wrote:
> >> Hi Erin,
> >>
> >> On 17/05/18 08:22, Erin Lo wrote:
> >>> MT8183 is a SoC based on 64bit ARMv8 architecture.
> >>> It contains 4 CA53 and 4 CA73 cores.
> >>> MT8183 share many HW IP with MT65xx series.
> >>> This patchset was tested on MT8183 evaluation board, and boot to shell ok.
> >>>
> >>> This series contains document bindings, device tree including interrupt, uart.
> >>>
> >>> Change in v3:
> >>> 1. Fill out GICC, GICH, GICV regions
> >>> 2. Update Copyright to 2018
> >>>
> >>> Change in v2:
> >>> 1. Split dt-bindings into different patches
> >>> 2. Correct bindings for supported SoCs (mtk-uart.txt)
> >>>
> >>> Ben Ho (1):
> >>> arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
> >>> Makefile
> >>>
> >>> Erin Lo (3):
> >>> dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
> >>> dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
> >>> dt-bindings: serial: Add compatible for Mediatek MT8183
> >>>
> >>
> >> I'm a bit reluctant to take this series, as it will only enable the EVB board to
> >> boot into a serial console. Are you planning to add support for other devices of
> >> this SoC?
> >>
> >> Apart please take into account that there is an issue with the dts file, as you
> >> were told by the kbuild test robot.
> >>
> >> Regards,
> >> Matthias
> >>
> >
> > Hi, Matthias
> > Sorry for missing this letter...since mail proxy server.
> > We plan to add support all the devices of MT8183 in serious.
> > We have implemented the clock and pinctrl driver for upstream and they
> > are in internal review right now.
> >
>
> Nice to hear that :)
>
> > About the dts issue... do you suggest me to send new patch right now or
> > wait for clock and pinctrl driver ready then send them together?
> >
>
> I would prefer that you send at least the clock controller together, so that we
> don't have any dummy clocks in the basic device tree.
>
> Regards,
> Matthias
>
Got it! Next patch, we will send them with clock controller without
dummy clocks in the basic device tree.
By the way, the clock controller driver of MT8183 is a little bit
different from former ICs, so we need more time to prepare them.
We will send them to public as soon as possible.
Best Regards,
Erin
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH] serial: 8250_ingenic: Add support for the JZ4725B SoC
From: Paul Cercueil @ 2018-07-20 18:58 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Mark Rutland, Jiri Slaby, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20180720153027.GA11978@rob-hp-laptop>
Hi Rob,
Le ven. 20 juil. 2018 à 17:30, Rob Herring <robh@kernel.org> a écrit :
> On Fri, Jul 13, 2018 at 04:38:40PM +0200, Paul Cercueil wrote:
>> The UART in the jz4725b works just like in the other JZ SoCs, so
>> this
>> commit simply adds a new compatible string.
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>> Documentation/devicetree/bindings/serial/ingenic,uart.txt | 1 +
>> drivers/tty/serial/8250/8250_ingenic.c | 5 +++++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/serial/ingenic,uart.txt
>> b/Documentation/devicetree/bindings/serial/ingenic,uart.txt
>> index c3c6406d5cfe..a0b34fd3b602 100644
>> --- a/Documentation/devicetree/bindings/serial/ingenic,uart.txt
>> +++ b/Documentation/devicetree/bindings/serial/ingenic,uart.txt
>> @@ -3,6 +3,7 @@
>> Required properties:
>> - compatible : One of:
>> - "ingenic,jz4740-uart",
>> + - "ingenic,jz4725b-uart",
>> - "ingenic,jz4760-uart",
>> - "ingenic,jz4770-uart",
>> - "ingenic,jz4775-uart",
>> diff --git a/drivers/tty/serial/8250/8250_ingenic.c
>> b/drivers/tty/serial/8250/8250_ingenic.c
>> index 15a8c8dfa92b..760266559a5a 100644
>> --- a/drivers/tty/serial/8250/8250_ingenic.c
>> +++ b/drivers/tty/serial/8250/8250_ingenic.c
>> @@ -133,6 +133,10 @@ EARLYCON_DECLARE(jz4740_uart,
>> ingenic_early_console_setup);
>> OF_EARLYCON_DECLARE(jz4740_uart, "ingenic,jz4740-uart",
>> ingenic_early_console_setup);
>>
>> +EARLYCON_DECLARE(jz4725b_uart, ingenic_early_console_setup);
>> +OF_EARLYCON_DECLARE(jz4725b_uart, "ingenic,jz4725b-uart",
>> + ingenic_early_console_setup);
>> +
>
> This shouldn't be necessary. Looks like it should be compatible with
> ingenic,jz4740-uart.
>
>> EARLYCON_DECLARE(jz4770_uart, ingenic_early_console_setup);
>> OF_EARLYCON_DECLARE(jz4770_uart, "ingenic,jz4770-uart",
>> ingenic_early_console_setup);
>> @@ -330,6 +334,7 @@ static const struct ingenic_uart_config
>> jz4780_uart_config = {
>>
>> static const struct of_device_id of_match[] = {
>> { .compatible = "ingenic,jz4740-uart", .data =
>> &jz4740_uart_config },
>> + { .compatible = "ingenic,jz4725b-uart", .data =
>> &jz4740_uart_config },
>
> And this too.
Well, I'm confused, the driver already uses multiple compatible strings
for SoCs that
work the exact same, so that was wrong?
>> { .compatible = "ingenic,jz4760-uart", .data =
>> &jz4760_uart_config },
>> { .compatible = "ingenic,jz4770-uart", .data =
>> &jz4760_uart_config },
>> { .compatible = "ingenic,jz4775-uart", .data =
>> &jz4760_uart_config },
>> --
>> 2.11.0
>>
^ permalink raw reply
* Re: [PATCH v5 04/11] soc: mediatek: add MT6765 scpsys support
From: Rob Herring @ 2018-07-20 17:46 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Ryder Lee, Stephen Boyd,
Sean Wang, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531817552-17221-5-git-send-email-mars.cheng@mediatek.com>
On Tue, Jul 17, 2018 at 04:52:25PM +0800, Mars Cheng wrote:
> This adds scpsys support for MT6765
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 88 ++++++++++++++++++++++++++++++
> include/dt-bindings/power/mt6765-power.h | 14 +++++
This file goes in the binding patch.
> 2 files changed, 102 insertions(+)
> create mode 100644 include/dt-bindings/power/mt6765-power.h
^ permalink raw reply
* Re: [PATCH v5 03/11] dt-bindings: mediatek: add MT6765 power dt-bindings
From: Rob Herring @ 2018-07-20 17:45 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Ryder Lee, Stephen Boyd,
Sean Wang, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531817552-17221-4-git-send-email-mars.cheng@mediatek.com>
On Tue, Jul 17, 2018 at 04:52:24PM +0800, Mars Cheng wrote:
> This adds power dt-bindings for MT6765
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> ---
> .../devicetree/bindings/soc/mediatek/scpsys.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 02/11] dt-bindings: mediatek: Add smi dts binding for Mediatek MT6765 SoC
From: Rob Herring @ 2018-07-20 17:44 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Ryder Lee, Stephen Boyd,
Sean Wang, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531817552-17221-3-git-send-email-mars.cheng@mediatek.com>
On Tue, Jul 17, 2018 at 04:52:23PM +0800, Mars Cheng wrote:
> This patch adds MT6765 smi binding document
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> ---
> .../memory-controllers/mediatek,smi-common.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 01/11] dt-bindings: clock: mediatek: document clk bindings for Mediatek MT6765 SoC
From: Rob Herring @ 2018-07-20 17:43 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Marc Zyngier, Ryder Lee, Stephen Boyd,
Sean Wang, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531817552-17221-2-git-send-email-mars.cheng@mediatek.com>
On Tue, Jul 17, 2018 at 04:52:22PM +0800, Mars Cheng wrote:
> This patch adds the binding documentation for apmixedsys, audsys, camsys,
> imgsys, infracfg, mipi0a, topckgen, vcodecsys
>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> ---
> .../bindings/arm/mediatek/mediatek,apmixedsys.txt | 1 +
> .../bindings/arm/mediatek/mediatek,audsys.txt | 1 +
> .../bindings/arm/mediatek/mediatek,camsys.txt | 27 ++++++++++++++++++++
> .../bindings/arm/mediatek/mediatek,imgsys.txt | 1 +
> .../bindings/arm/mediatek/mediatek,infracfg.txt | 1 +
> .../bindings/arm/mediatek/mediatek,mipi0a.txt | 23 +++++++++++++++++
> .../bindings/arm/mediatek/mediatek,mmsys.txt | 1 +
> .../bindings/arm/mediatek/mediatek,pericfg.txt | 1 +
> .../bindings/arm/mediatek/mediatek,topckgen.txt | 1 +
> .../bindings/arm/mediatek/mediatek,vcodecsys.txt | 22 ++++++++++++++++
> 10 files changed, 79 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mipi0a.txt
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vcodecsys.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> index b404d59..44eaeac 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,apmixedsys.txt
> @@ -8,6 +8,7 @@ Required Properties:
> - compatible: Should be one of:
> - "mediatek,mt2701-apmixedsys"
> - "mediatek,mt2712-apmixedsys", "syscon"
> + - "mediatek,mt6765-apmixedsys", "syscon"
> - "mediatek,mt6797-apmixedsys"
> - "mediatek,mt7622-apmixedsys"
> - "mediatek,mt8135-apmixedsys"
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> index 34a69ba..9a8672a 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,audsys.txt
> @@ -7,6 +7,7 @@ Required Properties:
>
> - compatible: Should be one of:
> - "mediatek,mt2701-audsys", "syscon"
> + - "mediatek,mt6765-audsys", "syscon"
> - "mediatek,mt7622-audsys", "syscon"
> - #clock-cells: Must be 1
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> new file mode 100644
> index 0000000..dc75783
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,camsys.txt
> @@ -0,0 +1,27 @@
> +MediaTek CAMSYS controller
> +============================
> +
> +The MediaTek CAMSYS controller provides various clocks to the system.
Only clocks? If so, then this should be moved to bindings/clocks/.
> +
> +Required Properties:
> +
> +- compatible: Should be one of:
> + - "mediatek,mt6765-camsys", "syscon"
> +- #clock-cells: Must be 1
> +
> +The AUDSYS controller uses the common clk binding from
> +Documentation/devicetree/bindings/clock/clock-bindings.txt
> +The available clocks are defined in dt-bindings/clock/mt*-clk.h.
> +
> +Required sub-nodes:
> +-------
> +For common binding part and usage, refer to
> +../sonud/mt2701-afe-pcm.txt.
> +
> +Example:
> +
> +camsys: camsys@1a000000 {
clock-controller@...
if the above answer is yes.
Same comments on the other docs.
^ permalink raw reply
* Re: [PATCH] serial: 8250_ingenic: Add support for the JZ4725B SoC
From: Rob Herring @ 2018-07-20 15:30 UTC (permalink / raw)
To: Paul Cercueil
Cc: Greg Kroah-Hartman, Mark Rutland, Jiri Slaby, linux-serial,
devicetree, linux-kernel
In-Reply-To: <20180713143840.9721-1-paul@crapouillou.net>
On Fri, Jul 13, 2018 at 04:38:40PM +0200, Paul Cercueil wrote:
> The UART in the jz4725b works just like in the other JZ SoCs, so this
> commit simply adds a new compatible string.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> Documentation/devicetree/bindings/serial/ingenic,uart.txt | 1 +
> drivers/tty/serial/8250/8250_ingenic.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/ingenic,uart.txt b/Documentation/devicetree/bindings/serial/ingenic,uart.txt
> index c3c6406d5cfe..a0b34fd3b602 100644
> --- a/Documentation/devicetree/bindings/serial/ingenic,uart.txt
> +++ b/Documentation/devicetree/bindings/serial/ingenic,uart.txt
> @@ -3,6 +3,7 @@
> Required properties:
> - compatible : One of:
> - "ingenic,jz4740-uart",
> + - "ingenic,jz4725b-uart",
> - "ingenic,jz4760-uart",
> - "ingenic,jz4770-uart",
> - "ingenic,jz4775-uart",
> diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
> index 15a8c8dfa92b..760266559a5a 100644
> --- a/drivers/tty/serial/8250/8250_ingenic.c
> +++ b/drivers/tty/serial/8250/8250_ingenic.c
> @@ -133,6 +133,10 @@ EARLYCON_DECLARE(jz4740_uart, ingenic_early_console_setup);
> OF_EARLYCON_DECLARE(jz4740_uart, "ingenic,jz4740-uart",
> ingenic_early_console_setup);
>
> +EARLYCON_DECLARE(jz4725b_uart, ingenic_early_console_setup);
> +OF_EARLYCON_DECLARE(jz4725b_uart, "ingenic,jz4725b-uart",
> + ingenic_early_console_setup);
> +
This shouldn't be necessary. Looks like it should be compatible with
ingenic,jz4740-uart.
> EARLYCON_DECLARE(jz4770_uart, ingenic_early_console_setup);
> OF_EARLYCON_DECLARE(jz4770_uart, "ingenic,jz4770-uart",
> ingenic_early_console_setup);
> @@ -330,6 +334,7 @@ static const struct ingenic_uart_config jz4780_uart_config = {
>
> static const struct of_device_id of_match[] = {
> { .compatible = "ingenic,jz4740-uart", .data = &jz4740_uart_config },
> + { .compatible = "ingenic,jz4725b-uart", .data = &jz4740_uart_config },
And this too.
> { .compatible = "ingenic,jz4760-uart", .data = &jz4760_uart_config },
> { .compatible = "ingenic,jz4770-uart", .data = &jz4760_uart_config },
> { .compatible = "ingenic,jz4775-uart", .data = &jz4760_uart_config },
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH v3 0/4] Add basic support for Mediatek MT8183 SoC
From: Matthias Brugger @ 2018-07-20 10:44 UTC (permalink / raw)
To: Erin Lo
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Greg Kroah-Hartman, devicetree, srv_heupstream,
mars.cheng, linux-kernel, linux-mediatek, linux-serial,
yingjoe.chen, linux-arm-kernel
In-Reply-To: <1532074769.19148.2.camel@mtksdaap41>
On 20/07/18 10:19, Erin Lo wrote:
> On Mon, 2018-07-16 at 11:28 +0200, Matthias Brugger wrote:
>> Hi Erin,
>>
>> On 17/05/18 08:22, Erin Lo wrote:
>>> MT8183 is a SoC based on 64bit ARMv8 architecture.
>>> It contains 4 CA53 and 4 CA73 cores.
>>> MT8183 share many HW IP with MT65xx series.
>>> This patchset was tested on MT8183 evaluation board, and boot to shell ok.
>>>
>>> This series contains document bindings, device tree including interrupt, uart.
>>>
>>> Change in v3:
>>> 1. Fill out GICC, GICH, GICV regions
>>> 2. Update Copyright to 2018
>>>
>>> Change in v2:
>>> 1. Split dt-bindings into different patches
>>> 2. Correct bindings for supported SoCs (mtk-uart.txt)
>>>
>>> Ben Ho (1):
>>> arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
>>> Makefile
>>>
>>> Erin Lo (3):
>>> dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
>>> dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
>>> dt-bindings: serial: Add compatible for Mediatek MT8183
>>>
>>
>> I'm a bit reluctant to take this series, as it will only enable the EVB board to
>> boot into a serial console. Are you planning to add support for other devices of
>> this SoC?
>>
>> Apart please take into account that there is an issue with the dts file, as you
>> were told by the kbuild test robot.
>>
>> Regards,
>> Matthias
>>
>
> Hi, Matthias
> Sorry for missing this letter...since mail proxy server.
> We plan to add support all the devices of MT8183 in serious.
> We have implemented the clock and pinctrl driver for upstream and they
> are in internal review right now.
>
Nice to hear that :)
> About the dts issue... do you suggest me to send new patch right now or
> wait for clock and pinctrl driver ready then send them together?
>
I would prefer that you send at least the clock controller together, so that we
don't have any dummy clocks in the basic device tree.
Regards,
Matthias
^ permalink raw reply
* Re: [PATCH v3 0/4] Add basic support for Mediatek MT8183 SoC
From: Erin Lo @ 2018-07-20 8:19 UTC (permalink / raw)
To: Matthias Brugger
Cc: Rob Herring, Mark Rutland, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Greg Kroah-Hartman, devicetree, srv_heupstream,
mars.cheng, linux-kernel, linux-mediatek, linux-serial,
yingjoe.chen, linux-arm-kernel
In-Reply-To: <9ba45353-a6e1-9a6e-f05b-4b1679ae9e52@gmail.com>
On Mon, 2018-07-16 at 11:28 +0200, Matthias Brugger wrote:
> Hi Erin,
>
> On 17/05/18 08:22, Erin Lo wrote:
> > MT8183 is a SoC based on 64bit ARMv8 architecture.
> > It contains 4 CA53 and 4 CA73 cores.
> > MT8183 share many HW IP with MT65xx series.
> > This patchset was tested on MT8183 evaluation board, and boot to shell ok.
> >
> > This series contains document bindings, device tree including interrupt, uart.
> >
> > Change in v3:
> > 1. Fill out GICC, GICH, GICV regions
> > 2. Update Copyright to 2018
> >
> > Change in v2:
> > 1. Split dt-bindings into different patches
> > 2. Correct bindings for supported SoCs (mtk-uart.txt)
> >
> > Ben Ho (1):
> > arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and
> > Makefile
> >
> > Erin Lo (3):
> > dt-bindings: arm: Add bindings for Mediatek MT8183 SoC Platform
> > dt-bindings: mtk-sysirq: Add compatible for Mediatek MT8183
> > dt-bindings: serial: Add compatible for Mediatek MT8183
> >
>
> I'm a bit reluctant to take this series, as it will only enable the EVB board to
> boot into a serial console. Are you planning to add support for other devices of
> this SoC?
>
> Apart please take into account that there is an issue with the dts file, as you
> were told by the kbuild test robot.
>
> Regards,
> Matthias
>
Hi, Matthias
Sorry for missing this letter...since mail proxy server.
We plan to add support all the devices of MT8183 in serious.
We have implemented the clock and pinctrl driver for upstream and they
are in internal review right now.
About the dts issue... do you suggest me to send new patch right now or
wait for clock and pinctrl driver ready then send them together?
Best Regards,
Erin
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH v2 1/2] tty/serial_core: add ISO7816 infrastructure
From: kbuild test robot @ 2018-07-19 23:54 UTC (permalink / raw)
Cc: linux-arch, alexandre.belloni, arnd, richard.genoud, gregkh,
linux-kernel, Ludovic Desroches, kbuild-all, linux-serial, jslaby,
linux-arm-kernel
In-Reply-To: <20180719084737.1490-2-ludovic.desroches@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 4392 bytes --]
Hi Nicolas,
I love your patch! Yet something to improve:
[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.18-rc5 next-20180719]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ludovic-Desroches/tty-serial_core-add-ISO7816-infrastructure/20180719-183102
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: parisc-generic-64bit_defconfig (attached as .config)
compiler: hppa64-linux-gnu-gcc (GCC) 7.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.3.0 make.cross ARCH=parisc
All errors (new ones prefixed by >>):
drivers/tty/serial/serial_core.c: In function 'uart_ioctl':
>> drivers/tty/serial/serial_core.c:1448:7: error: 'TIOCSISO7816' undeclared (first use in this function); did you mean 'TIOCSRS485'?
case TIOCSISO7816:
^~~~~~~~~~~~
TIOCSRS485
drivers/tty/serial/serial_core.c:1448:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/tty/serial/serial_core.c:1452:7: error: 'TIOCGISO7816' undeclared (first use in this function); did you mean 'TIOCSISO7816'?
case TIOCGISO7816:
^~~~~~~~~~~~
TIOCSISO7816
vim +1448 drivers/tty/serial/serial_core.c
1362
1363 /*
1364 * Called via sys_ioctl. We can use spin_lock_irq() here.
1365 */
1366 static int
1367 uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
1368 {
1369 struct uart_state *state = tty->driver_data;
1370 struct tty_port *port = &state->port;
1371 struct uart_port *uport;
1372 void __user *uarg = (void __user *)arg;
1373 int ret = -ENOIOCTLCMD;
1374
1375
1376 /*
1377 * These ioctls don't rely on the hardware to be present.
1378 */
1379 switch (cmd) {
1380 case TIOCGSERIAL:
1381 ret = uart_get_info_user(port, uarg);
1382 break;
1383
1384 case TIOCSSERIAL:
1385 down_write(&tty->termios_rwsem);
1386 ret = uart_set_info_user(tty, state, uarg);
1387 up_write(&tty->termios_rwsem);
1388 break;
1389
1390 case TIOCSERCONFIG:
1391 down_write(&tty->termios_rwsem);
1392 ret = uart_do_autoconfig(tty, state);
1393 up_write(&tty->termios_rwsem);
1394 break;
1395
1396 case TIOCSERGWILD: /* obsolete */
1397 case TIOCSERSWILD: /* obsolete */
1398 ret = 0;
1399 break;
1400 }
1401
1402 if (ret != -ENOIOCTLCMD)
1403 goto out;
1404
1405 if (tty_io_error(tty)) {
1406 ret = -EIO;
1407 goto out;
1408 }
1409
1410 /*
1411 * The following should only be used when hardware is present.
1412 */
1413 switch (cmd) {
1414 case TIOCMIWAIT:
1415 ret = uart_wait_modem_status(state, arg);
1416 break;
1417 }
1418
1419 if (ret != -ENOIOCTLCMD)
1420 goto out;
1421
1422 mutex_lock(&port->mutex);
1423 uport = uart_port_check(state);
1424
1425 if (!uport || tty_io_error(tty)) {
1426 ret = -EIO;
1427 goto out_up;
1428 }
1429
1430 /*
1431 * All these rely on hardware being present and need to be
1432 * protected against the tty being hung up.
1433 */
1434
1435 switch (cmd) {
1436 case TIOCSERGETLSR: /* Get line status register */
1437 ret = uart_get_lsr_info(tty, state, uarg);
1438 break;
1439
1440 case TIOCGRS485:
1441 ret = uart_get_rs485_config(uport, uarg);
1442 break;
1443
1444 case TIOCSRS485:
1445 ret = uart_set_rs485_config(uport, uarg);
1446 break;
1447
> 1448 case TIOCSISO7816:
1449 ret = uart_set_iso7816_config(state->uart_port, uarg);
1450 break;
1451
> 1452 case TIOCGISO7816:
1453 ret = uart_get_iso7816_config(state->uart_port, uarg);
1454 break;
1455 default:
1456 if (uport->ops->ioctl)
1457 ret = uport->ops->ioctl(uport, cmd, arg);
1458 break;
1459 }
1460 out_up:
1461 mutex_unlock(&port->mutex);
1462 out:
1463 return ret;
1464 }
1465
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19052 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/3] tty/serial_core: add ISO7816 infrastructure
From: Alan Cox @ 2018-07-19 11:07 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-arch, alexandre.belloni, arnd, richard.genoud, Greg KH,
linux-kernel, linux-serial, jslaby, linux-arm-kernel
In-Reply-To: <20180713080129.2ifw7crjksvaxxpq@M43218.corp.atmel.com>
> >
> > > + if (!port->iso7816_config)
> > > + return -ENOIOCTLCMD;
> >
> > Why this error value?
> >
>
> It was a mimic of RS485.
Which is what you want - it means the upper tty layer knows to offer the
ioctl to other places and then return appropriately.
Alan
^ permalink raw reply
* Re: [PATCH v2 0/2] add ISO7816 support
From: Ludovic Desroches @ 2018-07-19 9:26 UTC (permalink / raw)
To: Neil Armstrong
Cc: linux-serial, linux-arch, linux-arm-kernel, alexandre.belloni,
arnd, richard.genoud, gregkh, linux-kernel, jslaby
In-Reply-To: <034fe4ed-979b-51ee-db7c-5dc7137b98e5@baylibre.com>
Hi Neil,
On Thu, Jul 19, 2018 at 10:59:47AM +0200, Neil Armstrong wrote:
> Hi Ludovic,
>
> On 19/07/2018 10:47, Ludovic Desroches wrote:
> > Hi,
> >
> > This patchset adds support for the ISO7816 standard. The USART devices in
> > Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
> > the CLK and I/O signals of a smart card.
>
> Wow, I would have loved to have this at the time...
> I'm curious, do you have an example of userspace code using this ?
> The ATR rx needs a very weird handling, I'm curious how you managed it.
>
Unfortunately, I have nothing I can give you at the moment. I am doing
some experiments. Before going further, I need to have the interface
with the kernel accepted.
Of course, I can give more details about the experiments I am doing.
First of all, I am not used to ISO7816 so every feedback are
appreciated.
On the userspace part, there is the PCSC Lite library. It needs a
userspace driver. From what I've seen, most of the readers are managed
by the CCID driver. This driver handle different hardware capabilities.
Some hardware are very close to what we provide with our SoC ie. just
sending the data on the line and not manage the procedure bytes.
The CCID driver includes a lot of things related to the ISO7816 protocol
so I tried to plug my code into it to reuse this code. The issue is that I am
not a CCID device. At the moment, I am writing my own PCSC driver and I am
copying/pasting code I need. The USART only manages the CLK and I/O
signals. Others one are handled by the PCSC driver with the help of
GPIOs.
At a time, I was thinking about a CCID driver (which interprets the CCID
header and interacts with the device handling ISO7816) in the kernel to easily
interface with the PCSC CCID driver but I am not sure it's the right way
to go.
Regards
Ludovic
> Thanks,
> Neil
>
> >
> > Changes:
> > - v2
> > - uart_get_iso7816_config: check there is an iso7816_config function
> > - use IOCTL macros to generate the IOCTL number
> > - check that reserved field is not used
> > - remove debug logs
> > - check that the iso7816_config is right before doing any action
> > - change the error from nack and max iteration status to a debug message
> > - remove patch 3 as it concerns both rs485 and iso7816 to think more
> > about the need of adding a lock or not
> >
> > Nicolas Ferre (2):
> > tty/serial_core: add ISO7816 infrastructure
> > tty/serial: atmel: add ISO7816 support
> >
> > arch/alpha/include/uapi/asm/ioctls.h | 2 +
> > arch/mips/include/uapi/asm/ioctls.h | 2 +
> > arch/powerpc/include/uapi/asm/ioctls.h | 2 +
> > arch/sh/include/uapi/asm/ioctls.h | 2 +
> > arch/sparc/include/uapi/asm/ioctls.h | 2 +
> > arch/xtensa/include/uapi/asm/ioctls.h | 2 +
> > drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
> > drivers/tty/serial/atmel_serial.h | 3 +-
> > drivers/tty/serial/serial_core.c | 60 ++++++++++++
> > include/linux/serial_core.h | 3 +
> > include/uapi/asm-generic/ioctls.h | 2 +
> > include/uapi/linux/serial.h | 17 ++++
> > 12 files changed, 256 insertions(+), 11 deletions(-)
> >
>
^ permalink raw reply
* Re: [PATCH v2 0/2] add ISO7816 support
From: Neil Armstrong @ 2018-07-19 8:59 UTC (permalink / raw)
To: Ludovic Desroches, linux-serial, linux-arch, linux-arm-kernel
Cc: alexandre.belloni, arnd, richard.genoud, gregkh, linux-kernel,
jslaby
In-Reply-To: <20180719084737.1490-1-ludovic.desroches@microchip.com>
Hi Ludovic,
On 19/07/2018 10:47, Ludovic Desroches wrote:
> Hi,
>
> This patchset adds support for the ISO7816 standard. The USART devices in
> Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
> the CLK and I/O signals of a smart card.
Wow, I would have loved to have this at the time...
I'm curious, do you have an example of userspace code using this ?
The ATR rx needs a very weird handling, I'm curious how you managed it.
Thanks,
Neil
>
> Changes:
> - v2
> - uart_get_iso7816_config: check there is an iso7816_config function
> - use IOCTL macros to generate the IOCTL number
> - check that reserved field is not used
> - remove debug logs
> - check that the iso7816_config is right before doing any action
> - change the error from nack and max iteration status to a debug message
> - remove patch 3 as it concerns both rs485 and iso7816 to think more
> about the need of adding a lock or not
>
> Nicolas Ferre (2):
> tty/serial_core: add ISO7816 infrastructure
> tty/serial: atmel: add ISO7816 support
>
> arch/alpha/include/uapi/asm/ioctls.h | 2 +
> arch/mips/include/uapi/asm/ioctls.h | 2 +
> arch/powerpc/include/uapi/asm/ioctls.h | 2 +
> arch/sh/include/uapi/asm/ioctls.h | 2 +
> arch/sparc/include/uapi/asm/ioctls.h | 2 +
> arch/xtensa/include/uapi/asm/ioctls.h | 2 +
> drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
> drivers/tty/serial/atmel_serial.h | 3 +-
> drivers/tty/serial/serial_core.c | 60 ++++++++++++
> include/linux/serial_core.h | 3 +
> include/uapi/asm-generic/ioctls.h | 2 +
> include/uapi/linux/serial.h | 17 ++++
> 12 files changed, 256 insertions(+), 11 deletions(-)
>
^ permalink raw reply
* [PATCH v2 2/2] tty/serial: atmel: add ISO7816 support
From: Ludovic Desroches @ 2018-07-19 8:47 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
In-Reply-To: <20180719084737.1490-1-ludovic.desroches@microchip.com>
From: Nicolas Ferre <nicolas.ferre@microchip.com>
When mode is set in atmel_config_iso7816() we backup last RS232 mode
for coming back to this mode if requested.
Also allow setup of T=0 and T=1 parameter and basic support in set_termios
function as well.
Report NACK and ITER errors in irq handler.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++++++---
drivers/tty/serial/atmel_serial.h | 3 +-
2 files changed, 162 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 8e4428725848..cec958f1e7d4 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -34,6 +34,7 @@
#include <linux/suspend.h>
#include <linux/mm.h>
+#include <asm/div64.h>
#include <asm/io.h>
#include <asm/ioctls.h>
@@ -147,6 +148,8 @@ struct atmel_uart_port {
struct circ_buf rx_ring;
struct mctrl_gpios *gpios;
+ u32 backup_mode; /* MR saved during iso7816 operations */
+ u32 backup_brgr; /* BRGR saved during iso7816 operations */
unsigned int tx_done_mask;
u32 fifo_size;
u32 rts_high;
@@ -362,6 +365,132 @@ static int atmel_config_rs485(struct uart_port *port,
return 0;
}
+static unsigned int atmel_calc_cd(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int cd;
+ u64 mck_rate;
+
+ mck_rate = (u64)clk_get_rate(atmel_port->clk);
+ do_div(mck_rate, iso7816conf->clk);
+ cd = mck_rate;
+ return cd;
+}
+
+static unsigned int atmel_calc_fidi(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ u64 fidi = 0;
+
+ if (iso7816conf->sc_fi && iso7816conf->sc_di) {
+ fidi = (u64)iso7816conf->sc_fi;
+ do_div(fidi, iso7816conf->sc_di);
+ }
+ return (u32)fidi;
+}
+
+/* Enable or disable the iso7816 support */
+/* Called with interrupts disabled */
+static int atmel_config_iso7816(struct uart_port *port,
+ struct serial_iso7816 *iso7816conf)
+{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ unsigned int mode, t;
+ unsigned int cd, fidi;
+ int ret = 0;
+
+ /* Disable RX and TX */
+ atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXDIS | ATMEL_US_TXDIS);
+ /* Disable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
+
+ mode = atmel_uart_readl(port, ATMEL_US_MR);
+
+ if (iso7816conf->flags & SER_ISO7816_ENABLED) {
+ mode &= ~ATMEL_US_USMODE;
+
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T0;
+ t = 0;
+ } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(1)) {
+ mode |= ATMEL_US_USMODE_ISO7816_T1;
+ t = 1;
+ } else {
+ dev_warn(port->dev, "ISO7816 Type not supported. Resetting\n");
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ goto err_out;
+ }
+
+ dev_dbg(port->dev, "Setting USART to ISO7816 mode T%d\n", t);
+
+ mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
+ | ATMEL_US_NBSTOP | ATMEL_US_PAR);
+
+ /* NACK configuration */
+ if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0))
+ mode |= ATMEL_US_DSNACK;
+ else
+ mode |= ATMEL_US_INACK;
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set parity for normal/inverse mode + max iterations */
+ mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | (3 << 24);
+
+ cd = atmel_calc_cd(port, iso7816conf);
+ fidi = atmel_calc_fidi(port, iso7816conf);
+ if (fidi < 0) {
+ dev_warn(port->dev, "ISO7816 fidi = 0, Generator generates no signal\n");
+ } else if (fidi == 1 || fidi == 2) {
+ dev_err(port->dev, "ISO7816 fidi = %u, value not supported\n", fidi);
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED)) {
+ /* port not yet in iso7816 mode: store configuration */
+ atmel_port->backup_mode = atmel_uart_readl(port, ATMEL_US_MR);
+ atmel_port->backup_brgr = atmel_uart_readl(port, ATMEL_US_BRGR);
+ }
+
+ /* Actually set ISO7816 mode */
+ atmel_uart_writel(port, ATMEL_US_TTGR, iso7816conf->tg);
+ atmel_uart_writel(port, ATMEL_US_BRGR, cd);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, fidi);
+
+ atmel_port->tx_done_mask = ATMEL_US_TXEMPTY | ATMEL_US_NACK | ATMEL_US_ITERATION;
+ } else {
+ dev_dbg(port->dev, "Setting UART to RS232\n");
+ /* back to last RS232 settings */
+ mode = atmel_port->backup_mode;
+ memset(iso7816conf, 0, sizeof(struct serial_iso7816));
+ atmel_uart_writel(port, ATMEL_US_TTGR, 0);
+ atmel_uart_writel(port, ATMEL_US_BRGR, atmel_port->backup_brgr);
+ atmel_uart_writel(port, ATMEL_US_FIDIR, 0x174);
+
+ if (atmel_use_pdc_tx(port))
+ atmel_port->tx_done_mask = ATMEL_US_ENDTX |
+ ATMEL_US_TXBUFE;
+ else
+ atmel_port->tx_done_mask = ATMEL_US_TXRDY;
+ }
+
+ port->iso7816 = *iso7816conf;
+
+ atmel_uart_writel(port, ATMEL_US_MR, mode);
+
+err_out:
+ /* Enable interrupts */
+ atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
+ /* Enable RX and TX */
+ atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXEN | ATMEL_US_TXEN);
+
+ return ret;
+}
+
/*
* Return TIOCSER_TEMT when transmitter FIFO and Shift register is empty.
*/
@@ -481,8 +610,9 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_start_rx(port);
}
@@ -500,8 +630,9 @@ static void atmel_start_tx(struct uart_port *port)
return;
if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX))
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_stop_rx(port);
if (atmel_use_pdc_tx(port))
@@ -799,8 +930,9 @@ static void atmel_complete_tx_dma(void *arg)
*/
if (!uart_circ_empty(xmit))
atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
- else if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ else if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -1281,6 +1413,9 @@ atmel_handle_status(struct uart_port *port, unsigned int pending,
wake_up_interruptible(&port->state->port.delta_msr_wait);
}
}
+
+ if (pending & (ATMEL_US_NACK | ATMEL_US_ITERATION))
+ dev_dbg(port->dev, "ISO7816 ERROR (0x%08x)\n", pending);
}
/*
@@ -1373,8 +1508,9 @@ static void atmel_tx_pdc(struct uart_port *port)
atmel_uart_writel(port, ATMEL_US_IER,
atmel_port->tx_done_mask);
} else {
- if ((port->rs485.flags & SER_RS485_ENABLED) &&
- !(port->rs485.flags & SER_RS485_RX_DURING_TX)) {
+ if (((port->rs485.flags & SER_RS485_ENABLED) &&
+ !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
+ port->iso7816.flags & SER_ISO7816_ENABLED) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
@@ -2099,6 +2235,17 @@ 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 (port->iso7816.flags & SER_ISO7816_ENABLED) {
+ atmel_uart_writel(port, ATMEL_US_TTGR, port->iso7816.tg);
+ /* select mck clock, and output */
+ mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
+ /* set max iterations */
+ mode |= (3 << 24);
+ if ((port->iso7816.flags & SER_ISO7816_T_PARAM)
+ == SER_ISO7816_T(0))
+ mode |= ATMEL_US_USMODE_ISO7816_T0;
+ else
+ mode |= ATMEL_US_USMODE_ISO7816_T1;
} else if (termios->c_cflag & CRTSCTS) {
/* RS232 with hardware handshake (RTS/CTS) */
if (atmel_use_fifo(port) &&
@@ -2175,7 +2322,8 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
}
quot = cd | fp << ATMEL_US_FP_OFFSET;
- atmel_uart_writel(port, ATMEL_US_BRGR, quot);
+ if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
+ atmel_uart_writel(port, ATMEL_US_BRGR, quot);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
atmel_port->tx_stopped = false;
@@ -2355,6 +2503,7 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
port->mapbase = pdev->resource[0].start;
port->irq = pdev->resource[1].start;
port->rs485_config = atmel_config_rs485;
+ port->iso7816_config = atmel_config_iso7816;
port->membase = NULL;
memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
@@ -2379,7 +2528,8 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
}
/* Use TXEMPTY for interrupt when rs485 else TXRDY or ENDTX|TXBUFE */
- if (port->rs485.flags & SER_RS485_ENABLED)
+ if (port->rs485.flags & SER_RS485_ENABLED ||
+ port->iso7816.flags & SER_ISO7816_ENABLED)
atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
else if (atmel_use_pdc_tx(port)) {
port->fifosize = PDC_BUFFER_SIZE;
diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
index ba3a2437cde4..fff51f5fe8bc 100644
--- a/drivers/tty/serial/atmel_serial.h
+++ b/drivers/tty/serial/atmel_serial.h
@@ -124,7 +124,8 @@
#define ATMEL_US_TTGR 0x28 /* Transmitter Timeguard Register */
#define ATMEL_US_TG GENMASK(7, 0) /* Timeguard Value */
-#define ATMEL_US_FIDI 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDIR 0x40 /* FI DI Ratio Register */
+#define ATMEL_US_FIDI GENMASK(15, 0) /* FIDI ratio */
#define ATMEL_US_NER 0x44 /* Number of Errors Register */
#define ATMEL_US_IF 0x4c /* IrDA Filter Register */
--
2.12.2
^ permalink raw reply related
* [PATCH v2 1/2] tty/serial_core: add ISO7816 infrastructure
From: Ludovic Desroches @ 2018-07-19 8:47 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
In-Reply-To: <20180719084737.1490-1-ludovic.desroches@microchip.com>
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Add the ISO7816 ioctl and associated accessors and data structure.
Drivers can then use this common implementation to handle ISO7816.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Signed-off-by: Ludovic Desroches <ludovic.desroches@microchip.com>
---
arch/alpha/include/uapi/asm/ioctls.h | 2 ++
arch/mips/include/uapi/asm/ioctls.h | 2 ++
arch/powerpc/include/uapi/asm/ioctls.h | 2 ++
arch/sh/include/uapi/asm/ioctls.h | 2 ++
arch/sparc/include/uapi/asm/ioctls.h | 2 ++
arch/xtensa/include/uapi/asm/ioctls.h | 2 ++
drivers/tty/serial/serial_core.c | 60 ++++++++++++++++++++++++++++++++++
include/linux/serial_core.h | 3 ++
include/uapi/asm-generic/ioctls.h | 2 ++
include/uapi/linux/serial.h | 17 ++++++++++
10 files changed, 94 insertions(+)
diff --git a/arch/alpha/include/uapi/asm/ioctls.h b/arch/alpha/include/uapi/asm/ioctls.h
index 3729d92d3fa8..1e9121c9b3c7 100644
--- a/arch/alpha/include/uapi/asm/ioctls.h
+++ b/arch/alpha/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/mips/include/uapi/asm/ioctls.h b/arch/mips/include/uapi/asm/ioctls.h
index 890245a9f0c4..16aa8a766aec 100644
--- a/arch/mips/include/uapi/asm/ioctls.h
+++ b/arch/mips/include/uapi/asm/ioctls.h
@@ -93,6 +93,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
/* I hope the range from 0x5480 on is free ... */
#define TIOCSCTTY 0x5480 /* become controlling tty */
diff --git a/arch/powerpc/include/uapi/asm/ioctls.h b/arch/powerpc/include/uapi/asm/ioctls.h
index 41b1a5c15734..2c145da3b774 100644
--- a/arch/powerpc/include/uapi/asm/ioctls.h
+++ b/arch/powerpc/include/uapi/asm/ioctls.h
@@ -102,6 +102,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG 0x5453
#define TIOCSERGWILD 0x5454
diff --git a/arch/sh/include/uapi/asm/ioctls.h b/arch/sh/include/uapi/asm/ioctls.h
index cc62f6f98103..11866d4f60e1 100644
--- a/arch/sh/include/uapi/asm/ioctls.h
+++ b/arch/sh/include/uapi/asm/ioctls.h
@@ -95,6 +95,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG _IO('T', 83) /* 0x5453 */
#define TIOCSERGWILD _IOR('T', 84, int) /* 0x5454 */
diff --git a/arch/sparc/include/uapi/asm/ioctls.h b/arch/sparc/include/uapi/asm/ioctls.h
index 2df52711e170..7fd2f5873c9e 100644
--- a/arch/sparc/include/uapi/asm/ioctls.h
+++ b/arch/sparc/include/uapi/asm/ioctls.h
@@ -27,6 +27,8 @@
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGRS485 _IOR('T', 0x41, struct serial_rs485)
#define TIOCSRS485 _IOWR('T', 0x42, struct serial_rs485)
+#define TIOCGISO7816 _IOR('T', 0x43, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x44, struct serial_iso7816)
/* Note that all the ioctls that are not available in Linux have a
* double underscore on the front to: a) avoid some programs to
diff --git a/arch/xtensa/include/uapi/asm/ioctls.h b/arch/xtensa/include/uapi/asm/ioctls.h
index ec43609cbfc5..6d4a87296c95 100644
--- a/arch/xtensa/include/uapi/asm/ioctls.h
+++ b/arch/xtensa/include/uapi/asm/ioctls.h
@@ -107,6 +107,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define TIOCSERCONFIG _IO('T', 83)
#define TIOCSERGWILD _IOR('T', 84, int)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9c14a453f73c..af8d5b12fba9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1301,6 +1301,58 @@ static int uart_set_rs485_config(struct uart_port *port,
return 0;
}
+static int uart_get_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816)
+{
+ unsigned long flags;
+ struct serial_iso7816 aux;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ spin_lock_irqsave(&port->lock, flags);
+ aux = port->iso7816;
+ spin_unlock_irqrestore(&port->lock, flags);
+
+ if (copy_to_user(iso7816, &aux, sizeof(aux)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static int uart_set_iso7816_config(struct uart_port *port,
+ struct serial_iso7816 __user *iso7816_user)
+{
+ struct serial_iso7816 iso7816;
+ int i, ret;
+ unsigned long flags;
+
+ if (!port->iso7816_config)
+ return -ENOIOCTLCMD;
+
+ if (copy_from_user(&iso7816, iso7816_user, sizeof(*iso7816_user)))
+ return -EFAULT;
+
+ /*
+ * There are 5 words reserved for future use. Check that userspace
+ * doesn't put stuff in there to prevent breakages in the future.
+ */
+ for (i = 0; i < 5; i++)
+ if (iso7816.reserved[i])
+ return -EINVAL;
+
+ spin_lock_irqsave(&port->lock, flags);
+ ret = port->iso7816_config(port, &iso7816);
+ spin_unlock_irqrestore(&port->lock, flags);
+ if (ret)
+ return ret;
+
+ if (copy_to_user(iso7816_user, &port->iso7816, sizeof(port->iso7816)))
+ return -EFAULT;
+
+ return 0;
+}
+
/*
* Called via sys_ioctl. We can use spin_lock_irq() here.
*/
@@ -1385,6 +1437,14 @@ uart_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg)
case TIOCSRS485:
ret = uart_set_rs485_config(uport, uarg);
break;
+
+ case TIOCSISO7816:
+ ret = uart_set_iso7816_config(state->uart_port, uarg);
+ break;
+
+ case TIOCGISO7816:
+ ret = uart_get_iso7816_config(state->uart_port, uarg);
+ break;
default:
if (uport->ops->ioctl)
ret = uport->ops->ioctl(uport, cmd, arg);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 06ea4eeb09ab..d6e7747ffd46 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -137,6 +137,8 @@ struct uart_port {
void (*handle_break)(struct uart_port *);
int (*rs485_config)(struct uart_port *,
struct serial_rs485 *rs485);
+ int (*iso7816_config)(struct uart_port *,
+ struct serial_iso7816 *iso7816);
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
@@ -253,6 +255,7 @@ struct uart_port {
struct attribute_group *attr_group; /* port specific attributes */
const struct attribute_group **tty_groups; /* all attributes (serial core use only) */
struct serial_rs485 rs485;
+ struct serial_iso7816 iso7816;
void *private_data; /* generic platform data pointer */
};
diff --git a/include/uapi/asm-generic/ioctls.h b/include/uapi/asm-generic/ioctls.h
index 040651735662..cdc9f4ca8c27 100644
--- a/include/uapi/asm-generic/ioctls.h
+++ b/include/uapi/asm-generic/ioctls.h
@@ -79,6 +79,8 @@
#define TIOCGPTLCK _IOR('T', 0x39, int) /* Get Pty lock state */
#define TIOCGEXCL _IOR('T', 0x40, int) /* Get exclusive mode state */
#define TIOCGPTPEER _IO('T', 0x41) /* Safely open the slave */
+#define TIOCGISO7816 _IOR('T', 0x42, struct serial_iso7816)
+#define TIOCSISO7816 _IOWR('T', 0x43, struct serial_iso7816)
#define FIONCLEX 0x5450
#define FIOCLEX 0x5451
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 3fdd0dee8b41..93eb3c496ff1 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -132,4 +132,21 @@ struct serial_rs485 {
are a royal PITA .. */
};
+/*
+ * Serial interface for controlling ISO7816 settings on chips with suitable
+ * support. Set with TIOCSISO7816 and get with TIOCGISO7816 if supported by
+ * your platform.
+ */
+struct serial_iso7816 {
+ __u32 flags; /* ISO7816 feature flags */
+#define SER_ISO7816_ENABLED (1 << 0)
+#define SER_ISO7816_T_PARAM (0x0f << 4)
+#define SER_ISO7816_T(t) (((t) & 0x0f) << 4)
+ __u32 tg;
+ __u32 sc_fi;
+ __u32 sc_di;
+ __u32 clk;
+ __u32 reserved[5];
+};
+
#endif /* _UAPI_LINUX_SERIAL_H */
--
2.12.2
^ permalink raw reply related
* [PATCH v2 0/2] add ISO7816 support
From: Ludovic Desroches @ 2018-07-19 8:47 UTC (permalink / raw)
To: linux-serial, linux-arch, linux-arm-kernel
Cc: gregkh, jslaby, arnd, richard.genoud, nicolas.ferre,
alexandre.belloni, linux-kernel, Ludovic Desroches
Hi,
This patchset adds support for the ISO7816 standard. The USART devices in
Microchip SoCs have an ISO7816 mode. It allows to let the USART managing
the CLK and I/O signals of a smart card.
Changes:
- v2
- uart_get_iso7816_config: check there is an iso7816_config function
- use IOCTL macros to generate the IOCTL number
- check that reserved field is not used
- remove debug logs
- check that the iso7816_config is right before doing any action
- change the error from nack and max iteration status to a debug message
- remove patch 3 as it concerns both rs485 and iso7816 to think more
about the need of adding a lock or not
Nicolas Ferre (2):
tty/serial_core: add ISO7816 infrastructure
tty/serial: atmel: add ISO7816 support
arch/alpha/include/uapi/asm/ioctls.h | 2 +
arch/mips/include/uapi/asm/ioctls.h | 2 +
arch/powerpc/include/uapi/asm/ioctls.h | 2 +
arch/sh/include/uapi/asm/ioctls.h | 2 +
arch/sparc/include/uapi/asm/ioctls.h | 2 +
arch/xtensa/include/uapi/asm/ioctls.h | 2 +
drivers/tty/serial/atmel_serial.c | 170 +++++++++++++++++++++++++++++++--
drivers/tty/serial/atmel_serial.h | 3 +-
drivers/tty/serial/serial_core.c | 60 ++++++++++++
include/linux/serial_core.h | 3 +
include/uapi/asm-generic/ioctls.h | 2 +
include/uapi/linux/serial.h | 17 ++++
12 files changed, 256 insertions(+), 11 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: [PATCH v5 09/11] clk: mediatek: add new clkmux register API
From: Sean Wang @ 2018-07-19 6:57 UTC (permalink / raw)
To: Mars Cheng
Cc: Matthias Brugger, Rob Herring, Marc Zyngier, Ryder Lee,
Stephen Boyd, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531817552-17221-10-git-send-email-mars.cheng@mediatek.com>
On Tue, 2018-07-17 at 16:52 +0800, Mars Cheng wrote:
> From: Owen Chen <owen.chen@mediatek.com>
>
> MT6765 add "set/clr" register for each clkmux setting, and
> one update register to trigger value change. It is designed
> to prevent read-modify-write racing issue. The sw design
> need to add a new API to handle this hw change with a new
> mtk_clk_mux/mtk_clk_upd struct in new file "clk-mux"and
> clk-upd".
>
I don't see any word mtk_clk_upd or clk-upd in the patch
and the patch needs to be split into more patches
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> ---
> drivers/clk/mediatek/Makefile | 2 +-
> drivers/clk/mediatek/clk-mtk.c | 41 +++++++
> drivers/clk/mediatek/clk-mtk.h | 85 ++++++++++++---
> drivers/clk/mediatek/clk-mux.c | 236 ++++++++++++++++++++++++++++++++++++++++
> drivers/clk/mediatek/clk-mux.h | 38 +++++++
> 5 files changed, 388 insertions(+), 14 deletions(-)
> create mode 100644 drivers/clk/mediatek/clk-mux.c
> create mode 100644 drivers/clk/mediatek/clk-mux.h
>
> diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> index 844b55d..b97980d 100644
> --- a/drivers/clk/mediatek/Makefile
> +++ b/drivers/clk/mediatek/Makefile
> @@ -1,5 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> -obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o
> +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o
> obj-$(CONFIG_COMMON_CLK_MT6797) += clk-mt6797.o
> obj-$(CONFIG_COMMON_CLK_MT6797_IMGSYS) += clk-mt6797-img.o
> obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 9c0ae42..50becd0 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -22,6 +22,7 @@
> #include <linux/mfd/syscon.h>
>
> #include "clk-mtk.h"
> +#include "clk-mux.h"
> #include "clk-gate.h"
>
> struct clk_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
> @@ -144,6 +145,46 @@ int mtk_clk_register_gates(struct device_node *node,
> return 0;
> }
>
> +int mtk_clk_register_muxes(const struct mtk_mux *muxes,
> + int num, struct device_node *node,
> + spinlock_t *lock,
> + struct clk_onecell_data *clk_data)
> +{
> + struct regmap *regmap;
> + struct clk *clk;
> + int i;
> +
> + if (!clk_data)
> + return -ENOMEM;
> +
general register function is able to handle that there is no clk_data.
It looks like a optional, not a mandatory
> + regmap = syscon_node_to_regmap(node);
> + if (IS_ERR(regmap)) {
> + pr_err("Cannot find regmap for %pOF: %ld\n", node,
> + PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + for (i = 0; i < num; i++) {
> + const struct mtk_mux *mux = &muxes[i];
> +
> + if (clk_data && !IS_ERR_OR_NULL(clk_data->clks[mux->id]))
> + continue;
> +
it seems not necessary to check clk data every time
and always use positive check is good to read
> + clk = mtk_clk_register_mux(mux, regmap, lock);
> +
> + if (IS_ERR(clk)) {
> + pr_err("Failed to register clk %s: %ld\n",
> + mux->name, PTR_ERR(clk));
> + continue;
> + }
> +
> + if (clk_data)
> + clk_data->clks[mux->id] = clk;
don't alter any data from input, that is a surprise for users
> + }
> +
> + return 0;
> +}
> +
> struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
> void __iomem *base, spinlock_t *lock)
> {
> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> index 1882221..61693f6 100644
> --- a/drivers/clk/mediatek/clk-mtk.h
> +++ b/drivers/clk/mediatek/clk-mtk.h
> @@ -24,7 +24,9 @@
>
> #define MAX_MUX_GATE_BIT 31
> #define INVALID_MUX_GATE_BIT (MAX_MUX_GATE_BIT + 1)
> -
> +#define INVALID_OFS -1
> +#define INVALID_SHFT -1
> +#define INVALID_WIDTH -1
> #define MHZ (1000 * 1000)
>
> struct mtk_fixed_clk {
> @@ -84,10 +86,72 @@ struct mtk_composite {
> signed char num_parents;
> };
>
> +struct mtk_mux {
> + int id;
> + const char *name;
> + const char * const *parent_names;
> + unsigned int flags;
> +
> + u32 mux_ofs;
> + u32 set_ofs;
> + u32 clr_ofs;
> + u32 upd_ofs;
> +
> + signed char mux_shift;
> + signed char mux_width;
> + signed char gate_shift;
> + signed char upd_shift;
> +
> + const struct clk_ops *ops;
> +
> + signed char num_parents;
> +};
> +
you have created a mtk-mux.h, why is you don't move the newly create
struct in?
> /*
> * In case the rate change propagation to parent clocks is undesirable,
> * this macro allows to specify the clock flags manually.
> */
> +#define CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, _mux_set_ofs,\
> + _mux_clr_ofs, _shift, _width, _gate, \
> + _upd_ofs, _upd, _flags, _ops) { \
> + .id = _id, \
> + .name = _name, \
> + .mux_ofs = _mux_ofs, \
> + .set_ofs = _mux_set_ofs, \
> + .clr_ofs = _mux_clr_ofs, \
> + .upd_ofs = _upd_ofs, \
> + .mux_shift = _shift, \
> + .mux_width = _width, \
> + .gate_shift = _gate, \
> + .upd_shift = _upd, \
> + .parent_names = _parents, \
> + .num_parents = ARRAY_SIZE(_parents), \
> + .flags = _flags, \
> + .ops = &_ops, \
> + }
> +
> +#define MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, _mux_set_ofs,\
> + _mux_clr_ofs, _shift, _width, _gate, \
> + _upd_ofs, _upd, _flags) \
> + CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, _flags, \
> + mtk_mux_clr_set_upd_ops)
> +
> +#define MUX_CLR_SET_UPD(_id, _name, _parents, _mux_ofs, _mux_set_ofs, \
> + _mux_clr_ofs, _shift, _width, _gate, \
> + _upd_ofs, _upd) \
> + MUX_CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> + _mux_set_ofs, _mux_clr_ofs, _shift, _width, \
> + _gate, _upd_ofs, _upd, CLK_SET_RATE_PARENT)
> +
> +#define MUX_UPD(_id, _name, _parents, _mux_ofs, _shift, _width, _gate, \
> + _upd_ofs, _upd) \
> + CLR_SET_UPD_FLAGS(_id, _name, _parents, _mux_ofs, \
> + INVALID_OFS, INVALID_OFS, _shift, _width, \
> + _gate, _upd_ofs, _upd, CLK_SET_RATE_PARENT, \
> + mtk_mux_upd_ops)
> +
> #define MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, \
> _gate, _flags) { \
> .id = _id, \
> @@ -111,18 +175,8 @@ struct mtk_composite {
> MUX_GATE_FLAGS(_id, _name, _parents, _reg, _shift, _width, \
> _gate, CLK_SET_RATE_PARENT)
>
> -#define MUX(_id, _name, _parents, _reg, _shift, _width) { \
> - .id = _id, \
> - .name = _name, \
> - .mux_reg = _reg, \
> - .mux_shift = _shift, \
> - .mux_width = _width, \
> - .gate_shift = -1, \
> - .divider_shift = -1, \
> - .parent_names = _parents, \
> - .num_parents = ARRAY_SIZE(_parents), \
> - .flags = CLK_SET_RATE_PARENT, \
> - }
As Matthias always said that, you alter the common thing that is already
used by a lot of SoC.
You should have a dedicate patch to state why you need it, provide the
way you propose. and use another patches for migrating old SoC, finally
then add the patch for your own SoC.
Don't mix everything in a single patch. The patch can't become reusable
hard to read it, even hard to backport to the other SoC picking up
patches they are really wanting.
> +#define MUX(_id, _name, _parents, _reg, _shift, _width) \
> + MUX_GATE(_id, _name, _parents, _reg, _shift, _width, INVALID_SHFT)
>
> #define DIV_GATE(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg, \
> _div_width, _div_shift) { \
> @@ -138,6 +192,11 @@ struct mtk_composite {
> .flags = 0, \
> }
>
> +int mtk_clk_register_muxes(const struct mtk_mux *muxes,
> + int num, struct device_node *node,
> + spinlock_t *lock,
> + struct clk_onecell_data *clk_data);
> +
move to mtk-mux.h
> struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
> void __iomem *base, spinlock_t *lock);
>
> diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> new file mode 100644
> index 0000000..219181b
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mux.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + */
> +
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#include "clk-mtk.h"
> +#include "clk-mux.h"
> +
> +static inline struct mtk_clk_mux
> + *to_mtk_clk_mux(struct clk_hw *hw)
> +{
> + return container_of(hw, struct mtk_clk_mux, hw);
> +}
> +
> +static int mtk_mux_enable(struct clk_hw *hw)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 mask = BIT(mux->gate_shift);
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
we can see many functions in kernel, similar to your need, they always
be defined as two versions. for example.
mtk_mux_enable refer to lock version
mtk_mux_enable_nolock for lock-free version
that makes less condition, more readable, and users don't care much
about what stuff is put inside
> + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, 0);
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> + return 0;
> +}
> +
> +static void mtk_mux_disable(struct clk_hw *hw)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 mask = BIT(mux->gate_shift);
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
ditto
> + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, mask);
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> +}
> +
> +static int mtk_mux_enable_setclr(struct clk_hw *hw)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 val;
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
ditto
> + val = BIT(mux->gate_shift);
> + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> + return 0;
> +}
> +
> +static void mtk_mux_disable_setclr(struct clk_hw *hw)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 val;
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
ditto
> + val = BIT(mux->gate_shift);
> + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> +}
> +
> +static int mtk_mux_is_enabled(struct clk_hw *hw)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 val = 0;
> +
> + if (mux->gate_shift < 0)
> + return true;
> +
return value should be bool
> + regmap_read(mux->regmap, mux->mux_ofs, &val);
> +
> + return (val & BIT(mux->gate_shift)) == 0;
> +}
> +
> +static u8 mtk_mux_get_parent(struct clk_hw *hw)
> +{
return value should be int
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + int num_parents = clk_hw_get_num_parents(hw);
> + u32 mask = GENMASK(mux->mux_width - 1, 0);
> + u32 val;
> +
> + regmap_read(mux->regmap, mux->mux_ofs, &val);
> + val = (val >> mux->mux_shift) & mask;
> +
> + if (val >= num_parents)
> + return -EINVAL;
> +
> + return val;
> +}
> +
> +static int mtk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 mask = GENMASK(mux->mux_width - 1, 0);
> + u32 val, orig;
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
use lock-free-or-not funciton
> + regmap_read(mux->regmap, mux->mux_ofs, &val);
> + orig = val;
> + val &= ~(mask << mux->mux_shift);
> + val |= index << mux->mux_shift;
> +
> + if (val != orig) {
> + regmap_write(mux->regmap, mux->mux_ofs, val);
> +
> + if (mux->upd_shift >= 0)
> + regmap_write(mux->regmap, mux->upd_ofs,
> + BIT(mux->upd_shift));
why not use regmap_update_bits like function ?
> + }
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> +
> + return 0;
> +}
> +
> +static int mtk_mux_set_parent_setclr(struct clk_hw *hw, u8 index)
> +{
> + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> + u32 mask = GENMASK(mux->mux_width - 1, 0);
> + u32 val, orig;
> + unsigned long flags = 0;
> +
> + if (mux->lock)
> + spin_lock_irqsave(mux->lock, flags);
> +
use lock-free-or-not funciton
> + regmap_read(mux->regmap, mux->mux_ofs, &val);
> + orig = val;
> + val &= ~(mask << mux->mux_shift);
> + val |= index << mux->mux_shift;
> +
> + if (val != orig) {
> + val = (mask << mux->mux_shift);
> + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> + val = (index << mux->mux_shift);
> + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> +
why not use regmap_update_bits like function ?
> + if (mux->upd_shift >= 0)
> + regmap_write(mux->regmap, mux->upd_ofs,
> + BIT(mux->upd_shift));
> + }
> +
> + if (mux->lock)
> + spin_unlock_irqrestore(mux->lock, flags);
> +
> + return 0;
> +}
> +
> +const struct clk_ops mtk_mux_upd_ops = {
> + .enable = mtk_mux_enable,
> + .disable = mtk_mux_disable,
> + .is_enabled = mtk_mux_is_enabled,
> + .get_parent = mtk_mux_get_parent,
> + .set_parent = mtk_mux_set_parent,
> + .determine_rate = NULL,
explicitly set as NULL can be removed
> +};
> +
> +const struct clk_ops mtk_mux_clr_set_upd_ops = {
> + .enable = mtk_mux_enable_setclr,
> + .disable = mtk_mux_disable_setclr,
> + .is_enabled = mtk_mux_is_enabled,
> + .get_parent = mtk_mux_get_parent,
> + .set_parent = mtk_mux_set_parent_setclr,
> + .determine_rate = NULL,
explicitly set as NULL can be removed
> +};
> +
> +struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
> + struct regmap *regmap,
> + spinlock_t *lock)
> +{
> + struct clk *clk;
> + struct clk_init_data init;
> + struct mtk_clk_mux *mtk_mux = NULL;
> + int ret;
> +
make declaration as reverse xmas tree
> + mtk_mux = kzalloc(sizeof(*mtk_mux), GFP_KERNEL);
> + if (!mtk_mux)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = mux->name;
> + init.flags = (mux->flags) | CLK_SET_RATE_PARENT;
> + init.parent_names = mux->parent_names;
> + init.num_parents = mux->num_parents;
> + init.ops = mux->ops;
> +
> + mtk_mux->regmap = regmap;
> + mtk_mux->name = mux->name;
> + mtk_mux->mux_ofs = mux->mux_ofs;
> + mtk_mux->mux_set_ofs = mux->set_ofs;
> + mtk_mux->mux_clr_ofs = mux->clr_ofs;
> + mtk_mux->upd_ofs = mux->upd_ofs;
> + mtk_mux->mux_shift = mux->mux_shift;
> + mtk_mux->mux_width = mux->mux_width;
> + mtk_mux->gate_shift = mux->gate_shift;
> + mtk_mux->upd_shift = mux->upd_shift;
> +
> + mtk_mux->lock = lock;
> + mtk_mux->hw.init = &init;
> +
> + clk = clk_register(NULL, &mtk_mux->hw);
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
ret is superfluous
> + goto err_out;
> + }
> +
> + return clk;
> +err_out:
> + kfree(mtk_mux);
> +
I felt err path can be optimized
> + return ERR_PTR(ret);
> +}
> diff --git a/drivers/clk/mediatek/clk-mux.h b/drivers/clk/mediatek/clk-mux.h
> new file mode 100644
> index 0000000..64f8e7c
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mux.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + */
> +
> +#ifndef __DRV_CLK_MUX_H
> +#define __DRV_CLK_MUX_H
> +
> +#include <linux/clk-provider.h>
> +
> +struct mtk_clk_mux {
> + struct clk_hw hw;
> + struct regmap *regmap;
> +
> + const char *name;
> +
> + int mux_set_ofs;
> + int mux_clr_ofs;
> + int mux_ofs;
> + int upd_ofs;
> +
> + s8 mux_shift;
> + s8 mux_width;
> + s8 gate_shift;
> + s8 upd_shift;
> +
> + spinlock_t *lock;
> +};
> +
> +extern const struct clk_ops mtk_mux_upd_ops;
> +extern const struct clk_ops mtk_mux_clr_set_upd_ops;
> +
extern is superfluous
> +struct clk *mtk_clk_register_mux(const struct mtk_mux *mux,
> + struct regmap *regmap,
> + spinlock_t *lock);
> +
> +#endif /* __DRV_CLK_MUX_H */
^ permalink raw reply
* Re: [PATCH v1 0/3] console, serial8250: Disable PM and DMA ops
From: Tony Lindgren @ 2018-07-19 6:50 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Sebastian Andrzej Siewior, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt, Linux Kernel Mailing List,
Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Arnd Bergmann
In-Reply-To: <9bc02408588716c4ddb5a2ffb7d915019433fedc.camel@linux.intel.com>
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> [180718 14:54]:
> On Wed, 2018-05-23 at 11:00 -0700, Tony Lindgren wrote:
> > I thought you said earlier the issue was that you wanted to keep
> > the console enabled all the time and never idle?
>
> Yes, for kernel console.
> To be clear, if user supplies "console=ttySx" it keeps powered on
> always. But if there is no such parameter, we are fine with RPM.
I'm fine with that as long as we have a way to detach or disable
kernel console from userspace after booting so the console uart can
idle.
> Letting kernel console do power management on the systems without
> irq_safe hack is dangerous in terms of loosing important data (crash, or
> some other stuff which needs atomic context: kgdb?).
Yes in the long run we should get rid of all pm_runtime_irq_safe()
use. It increases the parent dev PM runtime usage count permanently
blocking the whole domain from idling.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v1 0/3] console, serial8250: Disable PM and DMA ops
From: Tony Lindgren @ 2018-07-19 6:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andy Shevchenko, Sebastian Andrzej Siewior, Petr Mladek,
Sergey Senozhatsky, Steven Rostedt, Linux Kernel Mailing List,
Greg Kroah-Hartman, Jiri Slaby, open list:SERIAL DRIVERS,
Arnd Bergmann
In-Reply-To: <c10bb604a6bf416e2092ae64a2cb9bb4658ea0eb.camel@linux.intel.com>
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> [180718 15:17]:
> On Wed, 2018-05-23 at 10:58 -0700, Tony Lindgren wrote:
> >
> > OK yeah console.idle sounds good to me. We should default to a
> > safe option.
>
> I'll see what we can do here.
Like we discussed offline I think if we allow detaching and attaching
kernel console from userspace we can get rid of irqsafe completely :)
And in that case no need for console.idle or anything like that.
Then if kernel console is attached, we can keep console uart enabled.
If detached, it can do runtime PM if hardware supports it.
Not sure if we need some separate new /sys entry for that, maybe
we already have that with the loglevel in /proc/sys/kernel/printk?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v1 0/3] console, serial8250: Disable PM and DMA ops
From: Andy Shevchenko @ 2018-07-18 15:14 UTC (permalink / raw)
To: Tony Lindgren, Andy Shevchenko
Cc: Sebastian Andrzej Siewior, Petr Mladek, Sergey Senozhatsky,
Steven Rostedt, Linux Kernel Mailing List, Greg Kroah-Hartman,
Jiri Slaby, open list:SERIAL DRIVERS, Arnd Bergmann
In-Reply-To: <20180523175859.GO98604@atomide.com>
On Wed, 2018-05-23 at 10:58 -0700, Tony Lindgren wrote:
> * Andy Shevchenko <andy.shevchenko@gmail.com> [180522 21:42]:
> > On Thu, May 17, 2018 at 10:30 PM, Tony Lindgren <tony@atomide.com>
> > wrote:
> > > So how about add some "noidle" kernel command line parameter for
> > > console
> > > that calls
> > > pm_runtime_forbid() and then you have the UART permanently
> > > on.
> >
> > IIUC _forbid() can be overwritten via sysfs.
> > And I would prefer to do other way around, something like
> > console.idle
> > and put default for OMAP to yes and no for everything else.
>
> OK yeah console.idle sounds good to me. We should default to a
> safe option.
I'll see what we can do here.
> > > Hmm I guess you could make also serial8250_rpm_get() do nothing
> > > based on that.
> >
> > Have you seen entire series which I keep here:
> > https://bitbucket.org/andy-shev/linux/branch/topic/uart/rpm?
> > Among other things it gets rid of those specific callbacks entirely.
>
> Well I was not Cc:ed on it, I browsed it in some archive and it
> seemed unsafe to me. But if you figured out a way to do it
> conditionally
> based on console.idle
> without causing regressions.
I restored that branch with some updated patches. It's far from done and
doesn't have any new stuff (yet) regrading to this discussion.
> > > I do agree the serial runtime PM has an issue if it depends on
> > > pm_runtime_irq_safe() being set.
> >
> > It's more than an issue. The so called "support" of RPM for UART is
> > _based on the hack_.
> > I would love to NAK that in the first place if I would have known of
> > it in time.
>
> Hmm well it seems that you too have been patching the 8250_rpm
> functions for years and then now what after multiple years you
> hit this issue? :)
Nope, I hit it as soon as I tried.
I can't find easily the discussion (hmm... yes, I was so pissed off that
time, I put a bit of harsh in that) I had with Sebastian few years back,
but at least I reported about an issue.
> > > > So, I can, of course just remove callbacks from the console
> > > > ->write().
> > > > Though it will prevent to use kernel console anyway.
> > >
> > > Please et's not start breaking things, we already see a constant
> > > flow of regressions on weekly basis.
> >
> > Now we are stick with a hack and the case based on that is against
> > fixing things.
> > This is how it looks from my side.
>
> Sorry yeah I agree there are issues, but let's fix it properly
Agree.
> with
> no regressions.
...though I think a word "regression" is inappropriate here. Regression
is what the support did in the first place. Pity I didn't know about it
at that time.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v1 0/3] console, serial8250: Disable PM and DMA ops
From: Andy Shevchenko @ 2018-07-18 14:50 UTC (permalink / raw)
To: Tony Lindgren, Andy Shevchenko
Cc: Sebastian Andrzej Siewior, Petr Mladek, Sergey Senozhatsky,
Steven Rostedt, Linux Kernel Mailing List, Greg Kroah-Hartman,
Jiri Slaby, open list:SERIAL DRIVERS, Arnd Bergmann
In-Reply-To: <20180523180052.GP98604@atomide.com>
On Wed, 2018-05-23 at 11:00 -0700, Tony Lindgren wrote:
> * Andy Shevchenko <andy.shevchenko@gmail.com> [180522 21:54]:
> > On Thu, May 17, 2018 at 10:48 PM, Tony Lindgren <tony@atomide.com>
> > wrote:
> > > * Andy Shevchenko <andy.shevchenko@gmail.com> [180517 16:38]:
> > > > On Thu, May 17, 2018 at 4:48 PM, Tony Lindgren <tony@atomide.com
> > > > > wrote:
> > > > > * Sebastian Andrzej Siewior <bigeasy@linutronix.de> [180516
> > > > > 10:49]:
> > > > > The idea breaking PM seems silly to me considering that we've
> > > > > had
> > > > > it working for years already.
> > > >
> > > > Same question / note. World is much more complex than just
> > > > being OMAP.
> > >
> > > Sorry but you are making assumptions about hardware being powered
> > > on
> > > all the time.
> >
> > Nope, other way around. The so called "support" _prevents_ our
> > hardware to go to sleep.
>
> Hmm sorry now I'm all confused what issues you're having.
>
> I thought you said earlier the issue was that you wanted to keep
> the console enabled all the time and never idle?
Yes, for kernel console.
To be clear, if user supplies "console=ttySx" it keeps powered on
always. But if there is no such parameter, we are fine with RPM.
Letting kernel console do power management on the systems without
irq_safe hack is dangerous in terms of loosing important data (crash, or
some other stuff which needs atomic context: kgdb?).
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH v5 06/11] soc: mediatek: add new flow for mtcmos power.
From: Matthias Brugger @ 2018-07-18 14:50 UTC (permalink / raw)
To: Mars Cheng, Rob Herring, Marc Zyngier, Ryder Lee, Stephen Boyd,
Sean Wang
Cc: CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, linux-clk,
Owen Chen
In-Reply-To: <1531817552-17221-7-git-send-email-mars.cheng@mediatek.com>
On 17/07/18 10:52, Mars Cheng wrote:
> From: Owen Chen <owen.chen@mediatek.com>
>
> MT6765 need multiple register and actions to setup bus
> protect.
> 1. turn on subsys CG before release bus protect to receive
> ack.
> 2. turn off subsys CG after set bus protect and receive
> ack.
> 3. bus protect need not only infracfg but other domain
> register to setup. Therefore we add a set/clr APIs
> with more customize arguments.
>
> Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> ---
> drivers/soc/mediatek/Makefile | 2 +-
> drivers/soc/mediatek/mtk-infracfg.c | 178 +++++++++++---
> drivers/soc/mediatek/mtk-scpsys-ext.c | 405 +++++++++++++++++++++++++++++++
> drivers/soc/mediatek/mtk-scpsys.c | 147 +++++++++--
> include/linux/soc/mediatek/infracfg.h | 9 +-
> include/linux/soc/mediatek/scpsys-ext.h | 66 +++++
> 6 files changed, 745 insertions(+), 62 deletions(-)
> create mode 100644 drivers/soc/mediatek/mtk-scpsys-ext.c
> create mode 100644 include/linux/soc/mediatek/scpsys-ext.h
>
> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> index 12998b0..9dc6670 100644
> --- a/drivers/soc/mediatek/Makefile
> +++ b/drivers/soc/mediatek/Makefile
> @@ -1,3 +1,3 @@
> -obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> +obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
> obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> diff --git a/drivers/soc/mediatek/mtk-infracfg.c b/drivers/soc/mediatek/mtk-infracfg.c
> index 958861c..11eadf8 100644
> --- a/drivers/soc/mediatek/mtk-infracfg.c
> +++ b/drivers/soc/mediatek/mtk-infracfg.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015 Pengutronix, Sascha Hauer <kernel@pengutronix.de>
> *
> @@ -15,6 +16,7 @@
> #include <linux/jiffies.h>
> #include <linux/regmap.h>
> #include <linux/soc/mediatek/infracfg.h>
> +#include <linux/soc/mediatek/scpsys-ext.h>
> #include <asm/processor.h>
>
> #define MTK_POLL_DELAY_US 10
> @@ -26,62 +28,176 @@
> #define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
>
> /**
> - * mtk_infracfg_set_bus_protection - enable bus protection
> - * @regmap: The infracfg regmap
> - * @mask: The mask containing the protection bits to be enabled.
> - * @reg_update: The boolean flag determines to set the protection bits
> - * by regmap_update_bits with enable register(PROTECTEN) or
> - * by regmap_write with set register(PROTECTEN_SET).
> + * mtk_generic_set_cmd - enable bus protection with set register
> + * @regmap: The bus protect regmap
> + * @set_ofs: The set register offset to set corresponding bit to 1.
> + * @sta_ofs: The status register offset to show bus protect enable/disable.
> + * @mask: The mask containing the protection bits to be disabled.
> *
> * This function enables the bus protection bits for disabled power
> * domains so that the system does not hang when some unit accesses the
> * bus while in power down.
> */
> -int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> - bool reg_update)
> +int mtk_generic_set_cmd(struct regmap *regmap, u32 set_ofs,
> + u32 sta_ofs, u32 mask)
> {
> u32 val;
> int ret;
>
> - if (reg_update)
> - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask,
> - mask);
> - else
> - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_SET, mask);
> + regmap_write(regmap, set_ofs, mask);
>
> - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> - val, (val & mask) == mask,
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = regmap_read_poll_timeout(regmap, sta_ofs, val,
> + (val & mask) == mask,
> + MTK_POLL_DELAY_US,
> + MTK_POLL_TIMEOUT);
>
> return ret;
> }
>
> /**
> - * mtk_infracfg_clear_bus_protection - disable bus protection
> - * @regmap: The infracfg regmap
> + * mtk_generic_clr_cmd - disable bus protection with clr register
> + * @regmap: The bus protect regmap
> + * @clr_ofs: The clr register offset to clear corresponding bit to 0.
> + * @sta_ofs: The status register offset to show bus protect enable/disable.
> * @mask: The mask containing the protection bits to be disabled.
> - * @reg_update: The boolean flag determines to clear the protection bits
> - * by regmap_update_bits with enable register(PROTECTEN) or
> - * by regmap_write with clear register(PROTECTEN_CLR).
> *
> * This function disables the bus protection bits previously enabled with
> - * mtk_infracfg_set_bus_protection.
> + * mtk_set_bus_protection.
> */
>
> -int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> - bool reg_update)
> +int mtk_generic_clr_cmd(struct regmap *regmap, u32 clr_ofs,
> + u32 sta_ofs, u32 mask)
> {
> int ret;
> u32 val;
>
> - if (reg_update)
> - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, 0);
> - else
> - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_CLR, mask);
> + regmap_write(regmap, clr_ofs, mask);
>
> - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> - val, !(val & mask),
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = regmap_read_poll_timeout(regmap, sta_ofs, val,
> + !(val & mask),
> + MTK_POLL_DELAY_US,
> + MTK_POLL_TIMEOUT);
> + return ret;
> +}
> +
> +/**
> + * mtk_generic_enable_cmd - enable bus protection with upd register
> + * @regmap: The bus protect regmap
> + * @upd_ofs: The update register offset to directly rewrite value to
> + * corresponding bit.
> + * @sta_ofs: The status register offset to show bus protect enable/disable.
> + * @mask: The mask containing the protection bits to be disabled.
> + *
> + * This function enables the bus protection bits for disabled power
> + * domains so that the system does not hang when some unit accesses the
> + * bus while in power down.
> + */
> +int mtk_generic_enable_cmd(struct regmap *regmap, u32 upd_ofs,
> + u32 sta_ofs, u32 mask)
> +{
> + u32 val;
> + int ret;
> +
> + regmap_update_bits(regmap, upd_ofs, mask, mask);
>
> + ret = regmap_read_poll_timeout(regmap, sta_ofs, val,
> + (val & mask) == mask,
> + MTK_POLL_DELAY_US,
> + MTK_POLL_TIMEOUT);
> return ret;
> }
> +
> +/**
> + * mtk_generic_disable_cmd - disable bus protection with updd register
> + * @regmap: The bus protect regmap
> + * @upd_ofs: The update register offset to directly rewrite value to
> + * corresponding bit.
> + * @sta_ofs: The status register offset to show bus protect enable/disable.
> + * @mask: The mask containing the protection bits to be disabled.
> + *
> + * This function disables the bus protection bits previously enabled with
> + * mtk_set_bus_protection.
> + */
> +
> +int mtk_generic_disable_cmd(struct regmap *regmap, u32 upd_ofs,
> + u32 sta_ofs, u32 mask)
> +{
> + int ret;
> + u32 val;
> +
> + regmap_update_bits(regmap, upd_ofs, mask, 0);
> +
> + ret = regmap_read_poll_timeout(regmap, sta_ofs,
> + val, !(val & mask),
> + MTK_POLL_DELAY_US,
> + MTK_POLL_TIMEOUT);
> + return ret;
> +}
> +
> +/**
> + * mtk_set_bus_protection - enable bus protection
> + * @infracfg: The bus protect regmap, default use infracfg
> + * @mask: The mask containing the protection bits to be enabled.
> + *
> + * This function enables the bus protection bits for disabled power
> + * domains so that the system does not hang when some unit accesses the
> + * bus while in power down.
> + */
> +int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask)
> +{
> + return mtk_generic_set_cmd(infracfg,
> + INFRA_TOPAXI_PROTECTEN_SET,
> + INFRA_TOPAXI_PROTECTSTA1,
> + mask);
> +}
> +
> +/**
> + * mtk_clear_bus_protection - disable bus protection
> + * @infracfg: The bus protect regmap, default use infracfg
> + * @mask: The mask containing the protection bits to be disabled.
> + *
> + * This function disables the bus protection bits previously enabled with
> + * mtk_set_bus_protection.
> + */
> +
> +int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask)
> +{
> + return mtk_generic_clr_cmd(infracfg,
> + INFRA_TOPAXI_PROTECTEN_CLR,
> + INFRA_TOPAXI_PROTECTSTA1,
> + mask);
> +}
> +
> +/**
> + * mtk_infracfg_enable_bus_protection - enable bus protection
> + * @infracfg: The bus protect regmap, default use infracfg
> + * @mask: The mask containing the protection bits to be disabled.
> + *
> + * This function enables the bus protection bits for disabled power
> + * domains so that the system does not hang when some unit accesses the
> + * bus while in power down.
> + */
> +int mtk_infracfg_enable_bus_protection(struct regmap *infracfg, u32 mask)
> +{
> + return mtk_generic_enable_cmd(infracfg,
> + INFRA_TOPAXI_PROTECTEN,
> + INFRA_TOPAXI_PROTECTSTA1,
> + mask);
> +}
> +
> +/**
> + * mtk_infracfg_disable_bus_protection - disable bus protection
> + * @infracfg: The bus protect regmap, default use infracfg
> + * @mask: The mask containing the protection bits to be disabled.
> + *
> + * This function disables the bus protection bits previously enabled with
> + * mtk_infracfg_set_bus_protection.
> + */
> +
> +int mtk_infracfg_disable_bus_protection(struct regmap *infracfg, u32 mask)
> +{
> + return mtk_generic_disable_cmd(infracfg,
> + INFRA_TOPAXI_PROTECTEN,
> + INFRA_TOPAXI_PROTECTSTA1,
> + mask);
> +}
> diff --git a/drivers/soc/mediatek/mtk-scpsys-ext.c b/drivers/soc/mediatek/mtk-scpsys-ext.c
> new file mode 100644
> index 0000000..965e64d
> --- /dev/null
> +++ b/drivers/soc/mediatek/mtk-scpsys-ext.c
> @@ -0,0 +1,405 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Owen Chen <owen.chen@mediatek.com>
> + */
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/slab.h>
> +#include <linux/export.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/soc/mediatek/infracfg.h>
> +#include <linux/soc/mediatek/scpsys-ext.h>
> +
> +
> +#define MAX_CLKS 10
> +#define INFRA "infracfg"
> +#define SMIC "smi_comm"
Don't use defines for this. While at it I suppose it should be "smi_common"
> +
> +static LIST_HEAD(ext_clk_map_list);
> +static LIST_HEAD(ext_attr_map_list);
> +
> +static struct regmap *infracfg;
> +static struct regmap *smi_comm;
> +
> +enum regmap_type {
> + IFR_TYPE,
> + SMI_TYPE,
> + MAX_REGMAP_TYPE,
> +};
> +
> +/**
> + * struct ext_reg_ctrl - set multiple register for bus protect
> + * @regmap: The bus protect regmap, 1: infracfg, 2: other master regmap
> + * such as SMI.
> + * @set_ofs: The set register offset to set corresponding bit to 1.
> + * @clr_ofs: The clr register offset to clear corresponding bit to 0.
> + * @sta_ofs: The status register offset to show bus protect enable/disable.
> + */
> +struct ext_reg_ctrl {
> + enum regmap_type type;
> + u32 set_ofs;
> + u32 clr_ofs;
> + u32 sta_ofs;
> +};
> +
> +/**
> + * struct ext_clk_ctrl - enable multiple clks for bus protect
> + * @clk: The clk need to enable before pwr on/bus protect.
> + * @scpd_n: The name present the scpsys domain where the clks belongs to.
> + * @clk_list: The list node linked to ext_clk_map_list.
> + */
> +struct ext_clk_ctrl {
> + struct clk *clk;
> + const char *scpd_n;
> + struct list_head clk_list;
> +};
> +
> +struct bus_mask_ops {
> + int (*set)(struct regmap *regmap, u32 set_ofs,
> + u32 sta_ofs, u32 mask);
> + int (*release)(struct regmap *regmap, u32 clr_ofs,
> + u32 sta_ofs, u32 mask);
> +};
> +
> +static struct scpsys_ext_attr *__get_attr_parent(const char *parent_n)
> +{
> + struct scpsys_ext_attr *attr;
> +
> + if (!parent_n)
> + return ERR_PTR(-EINVAL);
> +
> + list_for_each_entry(attr, &ext_attr_map_list, attr_list) {
> + if (attr->scpd_n && !strcmp(parent_n, attr->scpd_n))
> + return attr;
> + }
> +
> + return ERR_PTR(-EINVAL);
> +}
> +
> +int bus_ctrl_set_release(struct scpsys_ext_attr *attr, bool set)
> +{
> + int i;
> + int ret = 0;
> +
> + for (i = 0; i < MAX_STEP_NUM && attr->mask[i].mask; i++) {
> + struct ext_reg_ctrl *rc = attr->mask[i].regs;
> + struct regmap *regmap;
> +
> + if (rc->type == IFR_TYPE)
> + regmap = infracfg;
> + else if (rc->type == SMI_TYPE)
> + regmap = smi_comm;
> + else
> + return -EINVAL;
> +
> + if (set)
> + ret = attr->mask[i].ops->set(regmap,
> + rc->set_ofs,
> + rc->sta_ofs,
> + attr->mask[i].mask);
> + else
> + ret = attr->mask[i].ops->release(regmap,
> + rc->clr_ofs,
> + rc->sta_ofs,
> + attr->mask[i].mask);
> + }
> +
> + return ret;
> +}
> +
> +int bus_ctrl_set(struct scpsys_ext_attr *attr)
> +{
> + return bus_ctrl_set_release(attr, CMD_ENABLE);
> +}
> +
> +int bus_ctrl_release(struct scpsys_ext_attr *attr)
> +{
> + return bus_ctrl_set_release(attr, CMD_DISABLE);
> +}
> +
> +int bus_clk_enable_disable(struct scpsys_ext_attr *attr, bool enable)
> +{
> + int i = 0;
> + int ret = 0;
> + struct ext_clk_ctrl *cc;
> + struct clk *clk[MAX_CLKS];
> +
> + list_for_each_entry(cc, &ext_clk_map_list, clk_list) {
Why can't we handle this in the same way as we do in mtk-scpsys.c ?
> + if (!strcmp(cc->scpd_n, attr->scpd_n)) {
> + if (enable)
> + ret = clk_prepare_enable(cc->clk);
> + else
> + clk_disable_unprepare(cc->clk);
> +
> + if (ret) {
> + pr_err("Failed to %s %s\n",
> + enable ? "enable" : "disable",
> + __clk_get_name(cc->clk));
> + goto err;
> + } else {
> + clk[i] = cc->clk;
> + i++;
> + }
> + }
> + }
> +
> + return ret;
> +
> +err:
> + for (--i; i >= 0; i--)
> + if (enable)
> + clk_disable_unprepare(clk[i]);
> + else
> + clk_prepare_enable(clk[i]);
> + return ret;
> +}
> +
> +int bus_clk_enable(struct scpsys_ext_attr *attr)
> +{
> + struct scpsys_ext_attr *attr_p;
> + int ret = 0;
> +
> + attr_p = __get_attr_parent(attr->parent_n);
Why can't we implement this using the pg_genpd_add_subdomain approach?
> + if (!IS_ERR(attr_p)) {
> + ret = bus_clk_enable_disable(attr_p, CMD_ENABLE);
> + if (ret)
> + return ret;
> + }
> +
> + return bus_clk_enable_disable(attr, CMD_ENABLE);
> +}
> +
> +int bus_clk_disable(struct scpsys_ext_attr *attr)
> +{
> + struct scpsys_ext_attr *attr_p;
> + int ret = 0;
> +
> + ret = bus_clk_enable_disable(attr, CMD_DISABLE);
> + if (ret)
> + return ret;
> +
> + attr_p = __get_attr_parent(attr->parent_n);
> + if (!IS_ERR(attr_p))
> + ret = bus_clk_enable_disable(attr_p, CMD_DISABLE);
Same here.
> +
> + return ret;
> +}
> +
> +const struct bus_mask_ops bus_mask_set_clr_ctrl = {
> + .set = &mtk_generic_set_cmd,
> + .release = &mtk_generic_clr_cmd,
> +};
> +
> +const struct bus_ext_ops ext_bus_ctrl = {
> + .enable = &bus_ctrl_set,
> + .disable = &bus_ctrl_release,
> +};
> +
> +const struct bus_ext_ops ext_cg_ctrl = {
> + .enable = &bus_clk_enable,
> + .disable = &bus_clk_disable,
> +};
> +
> +/*
> + * scpsys bus driver init
> + */
> +struct regmap *syscon_regmap_lookup_by_phandle_idx(struct device_node *np,
> + const char *property,
> + int index)
> +{
> + struct device_node *syscon_np;
> + struct regmap *regmap;
> +
> + if (property)
> + syscon_np = of_parse_phandle(np, property, index);
> + else
> + syscon_np = np;
> +
> + if (!syscon_np)
> + return ERR_PTR(-ENODEV);
> +
> + regmap = syscon_node_to_regmap(syscon_np);
> + of_node_put(syscon_np);
> +
> + return regmap;
> +}
Why do we need this? It is never called...
> +
> +int scpsys_ext_regmap_init(struct platform_device *pdev)
> +{
> + infracfg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + INFRA);
> + if (IS_ERR(infracfg)) {
> + dev_err(&pdev->dev,
> + "Cannot find bus infracfg controller: %ld\n",
> + PTR_ERR(infracfg));
> + return PTR_ERR(infracfg);
> + }
> +
> + smi_comm = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + SMIC);
> + if (IS_ERR(smi_comm)) {
> + dev_err(&pdev->dev,
> + "Cannot find bus smi_comm controller: %ld\n",
> + PTR_ERR(smi_comm));
> + return PTR_ERR(smi_comm);
> + }
> +
> + return 0;
> +}
> +
> +static int add_clk_to_list(struct platform_device *pdev,
> + const char *name,
> + const char *scpd_n)
> +{
> + struct clk *clk;
> + struct ext_clk_ctrl *cc;
> +
> + clk = devm_clk_get(&pdev->dev, name);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Failed add clk %ld\n", PTR_ERR(clk));
> + return PTR_ERR(clk);
> + }
> +
> + cc = kzalloc(sizeof(*cc), GFP_KERNEL);
> + cc->clk = clk;
> + cc->scpd_n = kstrdup(scpd_n, GFP_KERNEL);
> +
> + list_add(&cc->clk_list, &ext_clk_map_list);
> +
> + return 0;
> +}
> +
> +static int add_cg_to_list(struct platform_device *pdev)
> +{
> + int i = 0;
> +
> + struct device_node *node = pdev->dev.of_node;
> +
> + if (!node) {
> + dev_err(&pdev->dev, "Cannot find topcksys node: %ld\n",
Why topcksys? Shouldn't that be the node of scpsys?
> + PTR_ERR(node));
> + return PTR_ERR(node);
> + }
> +
> + do {
> + const char *ck_name;
> + char *temp_str;
> + char *tok[2] = {NULL};
> + int cg_idx = 0;
> + int idx = 0;
> + int ret = 0;
> +
> + ret = of_property_read_string_index(node, "clock-names", i,
> + &ck_name);
> + if (ret < 0)
> + break;
> +
> + temp_str = kmalloc_array(strlen(ck_name), sizeof(char),
> + GFP_KERNEL | __GFP_ZERO);
> + memcpy(temp_str, ck_name, strlen(ck_name));
> + temp_str[strlen(ck_name)] = '\0';
why don't you use kstrdup or similar?
> + do {
> + tok[idx] = strsep(&temp_str, "-");
> + idx++;
> + } while (temp_str);
You want to split the clock name like "mm-2" in
char **tok = {"mm", "2"};
correct? That can be done easier AFAIK:
tok[0] = strsep(&temp_str, "-");
tok[1] = &temp_str;
> +
> + if (idx == 2) {
You don't add clocks like "mfg" and "mm". Why?
> + if (kstrtouint(tok[1], 10, &cg_idx))
And then? You don't do anything with cg_idx...
> + return -EINVAL;
> +
> + if (add_clk_to_list(pdev, ck_name, tok[0]))
add_clk_to_list third parameter is the name of the scp domain, but you pass the
clock name here. I'm puzzled.
> + return -EINVAL;
> + }
> + kfree(temp_str);
> + i++;
> + } while (1);
> +
> + return 0;
> +}
> +
> +int scpsys_ext_clk_init(struct platform_device *pdev)
> +{
> + int ret = 0;
> +
> + ret = add_cg_to_list(pdev);
> + if (ret)
> + goto err;
> +
> +err:
> + return ret;
> +}
Why do we need add_cg_to_list, it can be implemented directly here. Why is here
a goto to a return statement that will be executed anyway? Please go through the
code and check that it is clean before submitting.
> +
> +int scpsys_ext_attr_init(const struct scpsys_ext_data *data)
> +{
> + int i, count = 0;
> +
> + for (i = 0; i < data->num_attr; i++) {
> + struct scpsys_ext_attr *node = data->attr + i;
> +
> + if (!node)
> + continue;
> +
> + list_add(&node->attr_list, &ext_attr_map_list);
> + count++;
> + }
> +
> + if (!count)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id of_scpsys_ext_match_tbl[] = {
> + {
> + /* sentinel */
> + }
> +};
> +
> +struct scpsys_ext_data *scpsys_ext_init(struct platform_device *pdev)
> +{
> + const struct of_device_id *match;
> + struct scpsys_ext_data *data;
> + int ret;
> +
> + match = of_match_device(of_scpsys_ext_match_tbl, &pdev->dev);
> +
> + if (!match) {
> + dev_err(&pdev->dev, "no match\n");
> + return ERR_CAST(match);
> + }
> +
> + data = (struct scpsys_ext_data *)match->data;
> + if (IS_ERR(data)) {
> + dev_err(&pdev->dev, "no match scpext data\n");
> + return ERR_CAST(data);
> + }
> +
> + ret = scpsys_ext_attr_init(data);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Failed to init bus attr: %d\n",
> + ret);
> + return ERR_PTR(ret);
> + }
> +
> + ret = scpsys_ext_regmap_init(pdev);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "Failed to init bus register: %d\n",
> + ret);
> + return ERR_PTR(ret);
> + }
> +
> + ret = scpsys_ext_clk_init(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to init bus clks: %d\n",
> + ret);
> + return ERR_PTR(ret);
> + }
> +
> + return data;
> +}
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index 4bb6c7a..03df2d6 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2015 Pengutronix, Sascha Hauer <kernel@pengutronix.de>
> *
> @@ -20,6 +21,7 @@
> #include <linux/pm_domain.h>
> #include <linux/regulator/consumer.h>
> #include <linux/soc/mediatek/infracfg.h>
> +#include <linux/soc/mediatek/scpsys-ext.h>
>
> #include <dt-bindings/power/mt2701-power.h>
> #include <dt-bindings/power/mt2712-power.h>
> @@ -117,6 +119,15 @@ enum clk_id {
>
> #define MAX_CLKS 3
>
> +/**
> + * struct scp_domain_data - scp domain data for power on/off flow
> + * @name: The domain name.
> + * @sta_mask: The mask for power on/off status bit.
> + * @ctl_offs: The offset for main power control register.
> + * @sram_pdn_bits: The mask for sram power control bits.
> + * @sram_pdn_ack_bits The mask for sram power control acked bits.
> + * @caps: The flag for active wake-up action.
> + */
> struct scp_domain_data {
> const char *name;
> u32 sta_mask;
> @@ -150,7 +161,7 @@ struct scp {
> void __iomem *base;
> struct regmap *infracfg;
> struct scp_ctrl_reg ctrl_reg;
> - bool bus_prot_reg_update;
> + struct scpsys_ext_data *ext_data;
> };
>
> struct scp_subdomain {
> @@ -164,7 +175,6 @@ struct scp_soc_data {
> const struct scp_subdomain *subdomains;
> int num_subdomains;
> const struct scp_ctrl_reg regs;
> - bool bus_prot_reg_update;
> };
>
> static int scpsys_domain_is_on(struct scp_domain *scpd)
> @@ -236,6 +246,31 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> val |= PWR_RST_B_BIT;
> writel(val, ctl_addr);
>
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->cg_ops) {
> + ret = attr->cg_ops->enable(attr);
> + if (ret)
> + goto err_ext_clk;
> + }
> + }
> +
> + val &= ~scpd->data->sram_pdn_bits;
> + writel(val, ctl_addr);
> +
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->cg_ops) {
> + ret = attr->cg_ops->enable(attr);
> + if (ret)
> + goto err_ext_clk;
> + }
> + }
> +
> val &= ~scpd->data->sram_pdn_bits;
> writel(val, ctl_addr);
>
> @@ -247,25 +282,65 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> * applied here.
> */
> usleep_range(12000, 12100);
> -
> } else {
> ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> if (ret < 0)
> - goto err_pwr_ack;
> + goto err_sram;
> }
>
> if (scpd->data->bus_prot_mask) {
> ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> - scpd->data->bus_prot_mask,
> - scp->bus_prot_reg_update);
> + scpd->data->bus_prot_mask);
> if (ret)
> - goto err_pwr_ack;
> + goto err_sram;
> + }
> +
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->bus_ops) {
> + ret = attr->bus_ops->disable(attr);
> + if (ret)
> + goto err_sram;
> + }
> + }
> +
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->cg_ops) {
> + ret = attr->cg_ops->disable(attr);
> + if (ret)
> + goto err_sram;
> + }
> }
>
> return 0;
>
> +err_sram:
> + val = readl(ctl_addr);
> + val |= scpd->data->sram_pdn_bits;
> + writel(val, ctl_addr);
> +err_ext_clk:
> + val = readl(ctl_addr);
> + val |= PWR_ISO_BIT;
> + writel(val, ctl_addr);
> +
> + val &= ~PWR_RST_B_BIT;
> + writel(val, ctl_addr);
> +
> + val |= PWR_CLK_DIS_BIT;
> + writel(val, ctl_addr);
> err_pwr_ack:
> + val &= ~PWR_ON_BIT;
> + writel(val, ctl_addr);
> +
> + val &= ~PWR_ON_2ND_BIT;
> + writel(val, ctl_addr);
> +
> for (i = MAX_CLKS - 1; i >= 0; i--) {
> if (scpd->clk[i])
> clk_disable_unprepare(scpd->clk[i]);
> @@ -274,8 +349,6 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> if (scpd->supply)
> regulator_disable(scpd->supply);
>
> - dev_err(scp->dev, "Failed to power on domain %s\n", genpd->name);
> -
> return ret;
> }
>
> @@ -289,14 +362,35 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> int ret, tmp;
> int i;
>
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->cg_ops) {
> + ret = attr->cg_ops->enable(attr);
> + if (ret)
> + goto out;
> + }
> + }
> +
> if (scpd->data->bus_prot_mask) {
> ret = mtk_infracfg_set_bus_protection(scp->infracfg,
> - scpd->data->bus_prot_mask,
> - scp->bus_prot_reg_update);
> + scpd->data->bus_prot_mask);
> if (ret)
> goto out;
> }
>
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->bus_ops) {
> + ret = attr->bus_ops->enable(attr);
> + if (ret)
> + goto out;
> + }
> + }
> +
> val = readl(ctl_addr);
> val |= scpd->data->sram_pdn_bits;
> writel(val, ctl_addr);
> @@ -307,6 +401,17 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> if (ret < 0)
> goto out;
>
> + if (!IS_ERR(scp->ext_data)) {
> + struct scpsys_ext_attr *attr;
> +
> + attr = scp->ext_data->get_attr(scpd->data->name);
> + if (!IS_ERR(attr) && attr->cg_ops) {
> + ret = attr->cg_ops->disable(attr);
> + if (ret)
> + goto out;
> + }
> + }
> +
> val |= PWR_ISO_BIT;
> writel(val, ctl_addr);
>
> @@ -337,8 +442,6 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> return 0;
>
> out:
> - dev_err(scp->dev, "Failed to power off domain %s\n", genpd->name);
> -
> return ret;
> }
>
> @@ -352,8 +455,7 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
>
> static struct scp *init_scp(struct platform_device *pdev,
> const struct scp_domain_data *scp_domain_data, int num,
> - const struct scp_ctrl_reg *scp_ctrl_reg,
> - bool bus_prot_reg_update)
> + const struct scp_ctrl_reg *scp_ctrl_reg)
> {
> struct genpd_onecell_data *pd_data;
> struct resource *res;
> @@ -367,11 +469,10 @@ static struct scp *init_scp(struct platform_device *pdev,
>
> scp->ctrl_reg.pwr_sta_offs = scp_ctrl_reg->pwr_sta_offs;
> scp->ctrl_reg.pwr_sta2nd_offs = scp_ctrl_reg->pwr_sta2nd_offs;
> -
> - scp->bus_prot_reg_update = bus_prot_reg_update;
> -
> scp->dev = &pdev->dev;
>
> + scp->ext_data = scpsys_ext_init(pdev);
> +
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> scp->base = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(scp->base))
> @@ -1021,7 +1122,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt2712_data = {
> @@ -1033,7 +1133,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = false,
> };
>
> static const struct scp_soc_data mt6765_data = {
> @@ -1056,7 +1155,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS_MT6797,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
> },
> - .bus_prot_reg_update = true,
I don't understand why you can delete this flag if you don't change anything
else in the data structure. For me this looks like you will break other chips.
Please explain.
I have the gut feeling that this can be implemented in the existing mtk-scpsys
driver. Can you please explain what are the points that this is not possible.
I want to understand the design decisions you made here, because they seem
really odd to me.
Best regards,
Matthias
> };
>
> static const struct scp_soc_data mt7622_data = {
> @@ -1066,7 +1164,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt7623a_data = {
> @@ -1076,7 +1173,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt8173_data = {
> @@ -1088,7 +1184,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> /*
> @@ -1132,8 +1227,8 @@ static int scpsys_probe(struct platform_device *pdev)
>
> soc = of_device_get_match_data(&pdev->dev);
>
> - scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs,
> - soc->bus_prot_reg_update);
> + scp = init_scp(pdev, soc->domains, soc->num_domains,
> + &soc->regs);
> if (IS_ERR(scp))
> return PTR_ERR(scp);
>
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index fd25f01..bfad082 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -32,8 +32,9 @@
> #define MT7622_TOP_AXI_PROT_EN_WB (BIT(2) | BIT(6) | \
> BIT(7) | BIT(8))
>
> -int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> - bool reg_update);
> -int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> - bool reg_update);
> +int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask);
> +int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask);
> +int mtk_infracfg_enable_bus_protection(struct regmap *infracfg, u32 mask);
> +int mtk_infracfg_disable_bus_protection(struct regmap *infracfg, u32 mask);
> +
> #endif /* __SOC_MEDIATEK_INFRACFG_H */
> diff --git a/include/linux/soc/mediatek/scpsys-ext.h b/include/linux/soc/mediatek/scpsys-ext.h
> new file mode 100644
> index 0000000..99b5ff1
> --- /dev/null
> +++ b/include/linux/soc/mediatek/scpsys-ext.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __SOC_MEDIATEK_SCPSYS_EXT_H
> +#define __SOC_MEDIATEK_SCPSYS_EXT_H
> +
> +#include <linux/platform_device.h>
> +
> +#define CMD_ENABLE 1
> +#define CMD_DISABLE 0
> +
> +#define MAX_STEP_NUM 4
> +
> +/**
> + * struct bus_mask - set mask and corresponding operation for bus protect
> + * @regs: The register set of bus register control, including set/clr/sta.
> + * @mask: The mask set for bus protect.
> + * @flag: The flag to idetify which operation we take for bus protect.
> + */
> +struct bus_mask {
> + struct ext_reg_ctrl *regs;
> + u32 mask;
> + const struct bus_mask_ops *ops;
> +};
> +
> +/**
> + * struct scpsys_ext_attr - extended attribute for bus protect and further
> + * operand.
> + *
> + * @scpd_n: The name present the scpsys domain where the clks belongs to.
> + * @mask: The mask set for bus protect.
> + * @bus_ops: The operation we take for bus protect.
> + * @cg_ops: The operation we take for cg on/off.
> + * @attr_list: The list node linked to ext_attr_map_list.
> + */
> +struct scpsys_ext_attr {
> + const char *scpd_n;
> + struct bus_mask mask[MAX_STEP_NUM];
> + const char *parent_n;
> + const struct bus_ext_ops *bus_ops;
> + const struct bus_ext_ops *cg_ops;
> +
> + struct list_head attr_list;
> +};
> +
> +struct scpsys_ext_data {
> + struct scpsys_ext_attr *attr;
> + u8 num_attr;
> + struct scpsys_ext_attr * (*get_attr)(const char *scpd_n);
> +};
> +
> +struct bus_ext_ops {
> + int (*enable)(struct scpsys_ext_attr *attr);
> + int (*disable)(struct scpsys_ext_attr *attr);
> +};
> +
> +int mtk_generic_set_cmd(struct regmap *regmap, u32 set_ofs,
> + u32 sta_ofs, u32 mask);
> +int mtk_generic_clr_cmd(struct regmap *regmap, u32 clr_ofs,
> + u32 sta_ofs, u32 mask);
> +int mtk_generic_enable_cmd(struct regmap *regmap, u32 upd_ofs,
> + u32 sta_ofs, u32 mask);
> +int mtk_generic_disable_cmd(struct regmap *regmap, u32 upd_ofs,
> + u32 sta_ofs, u32 mask);
> +
> +struct scpsys_ext_data *scpsys_ext_init(struct platform_device *pdev);
> +
> +#endif /* __SOC_MEDIATEK_SCPSYS_EXT_H */
>
^ permalink raw reply
* Re: [PATCH v5 04/11] soc: mediatek: add MT6765 scpsys support
From: Mars Cheng @ 2018-07-18 8:54 UTC (permalink / raw)
To: Sean Wang
Cc: Matthias Brugger, Rob Herring, Marc Zyngier, Ryder Lee,
Stephen Boyd, CC Hwang, Loda Chou, linux-kernel, linux-mediatek,
devicetree, wsd_upstream, linux-serial, linux-arm-kernel,
linux-clk, Owen Chen
In-Reply-To: <1531832413.8953.30.camel@mtkswgap22>
Hi Sean
On Tue, 2018-07-17 at 21:00 +0800, Sean Wang wrote:
> On Tue, 2018-07-17 at 16:52 +0800, Mars Cheng wrote:
> > This adds scpsys support for MT6765
> >
>
> it looks like 4/11 have to depend on 6/11 and 7/11 to get a full function on scpsys for MT6765.
>
> you should keep dependency in order to submit these patches
>
Got it, will reorder the sequence.
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > Signed-off-by: Owen Chen <owen.chen@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 88 ++++++++++++++++++++++++++++++
> > include/dt-bindings/power/mt6765-power.h | 14 +++++
> > 2 files changed, 102 insertions(+)
> > create mode 100644 include/dt-bindings/power/mt6765-power.h
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index 5b24bb4..4bb6c7a 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -23,6 +23,7 @@
> >
> > #include <dt-bindings/power/mt2701-power.h>
> > #include <dt-bindings/power/mt2712-power.h>
> > +#include <dt-bindings/power/mt6765-power.h>
> > #include <dt-bindings/power/mt6797-power.h>
> > #include <dt-bindings/power/mt7622-power.h>
> > #include <dt-bindings/power/mt7623a-power.h>
> > @@ -680,6 +681,79 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > };
> >
> > /*
> > + * MT6765 power domain support
> > + */
> > +#define SPM_PWR_STATUS_MT6765 0x0180
> > +#define SPM_PWR_STATUS_2ND_MT6765 0x0184
> > +
> > +static const struct scp_domain_data scp_domain_data_mt6765[] = {
> > + [MT6765_POWER_DOMAIN_VCODEC] = {
> > + .name = "vcodec",
> > + .sta_mask = BIT(26),
> > + .ctl_offs = 0x300,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + },
> > + [MT6765_POWER_DOMAIN_ISP] = {
> > + .name = "isp",
> > + .sta_mask = BIT(5),
> > + .ctl_offs = 0x308,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + },
> > + [MT6765_POWER_DOMAIN_MM] = {
> > + .name = "mm",
> > + .sta_mask = BIT(3),
> > + .ctl_offs = 0x30C,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + },
> > + [MT6765_POWER_DOMAIN_CONN] = {
> > + .name = "conn",
> > + .sta_mask = BIT(1),
> > + .ctl_offs = 0x32C,
> > + .sram_pdn_bits = 0,
> > + .sram_pdn_ack_bits = 0,
> > + },
> > + [MT6765_POWER_DOMAIN_MFG_ASYNC] = {
> > + .name = "mfg_async",
> > + .sta_mask = BIT(23),
> > + .ctl_offs = 0x334,
> > + .sram_pdn_bits = 0,
> > + .sram_pdn_ack_bits = 0,
> > + },
> > + [MT6765_POWER_DOMAIN_MFG] = {
> > + .name = "mfg",
> > + .sta_mask = BIT(4),
> > + .ctl_offs = 0x338,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + },
> > + [MT6765_POWER_DOMAIN_CAM] = {
> > + .name = "cam",
> > + .sta_mask = BIT(25),
> > + .ctl_offs = 0x344,
> > + .sram_pdn_bits = GENMASK(8, 9),
> > + .sram_pdn_ack_bits = GENMASK(12, 13),
> > + },
> > + [MT6765_POWER_DOMAIN_MFG_CORE0] = {
> > + .name = "mfg_core0",
> > + .sta_mask = BIT(7),
> > + .ctl_offs = 0x34C,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + },
>
> Above power domains really don't require any clock controlled with
> clk_id before any access on them?
>
Yes, already sync with Owen, they are lost. will add them.
Thanks.
> > +};
> > +
> > +static const struct scp_subdomain scp_subdomain_mt6765[] = {
> > + {MT6765_POWER_DOMAIN_MM, MT6765_POWER_DOMAIN_CAM},
> > + {MT6765_POWER_DOMAIN_MM, MT6765_POWER_DOMAIN_ISP},
> > + {MT6765_POWER_DOMAIN_MM, MT6765_POWER_DOMAIN_VCODEC},
> > + {MT6765_POWER_DOMAIN_MFG_ASYNC, MT6765_POWER_DOMAIN_MFG},
> > + {MT6765_POWER_DOMAIN_MFG, MT6765_POWER_DOMAIN_MFG_CORE0},
> > +};
> > +
> > +/*
> > * MT6797 power domain support
> > */
> >
> > @@ -962,6 +1036,17 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .bus_prot_reg_update = false,
> > };
> >
> > +static const struct scp_soc_data mt6765_data = {
> > + .domains = scp_domain_data_mt6765,
> > + .num_domains = ARRAY_SIZE(scp_domain_data_mt6765),
> > + .subdomains = scp_subdomain_mt6765,
> > + .num_subdomains = ARRAY_SIZE(scp_subdomain_mt6765),
> > + .regs = {
> > + .pwr_sta_offs = SPM_PWR_STATUS_MT6765,
> > + .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6765,
> > + },
> > +};
> > +
> > static const struct scp_soc_data mt6797_data = {
> > .domains = scp_domain_data_mt6797,
> > .num_domains = ARRAY_SIZE(scp_domain_data_mt6797),
> > @@ -1018,6 +1103,9 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .compatible = "mediatek,mt2712-scpsys",
> > .data = &mt2712_data,
> > }, {
> > + .compatible = "mediatek,mt6765-scpsys",
> > + .data = &mt6765_data,
> > + }, {
> > .compatible = "mediatek,mt6797-scpsys",
> > .data = &mt6797_data,
> > }, {
> > diff --git a/include/dt-bindings/power/mt6765-power.h b/include/dt-bindings/power/mt6765-power.h
> > new file mode 100644
> > index 0000000..d347b4e
> > --- /dev/null
> > +++ b/include/dt-bindings/power/mt6765-power.h
> > @@ -0,0 +1,14 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _DT_BINDINGS_POWER_MT6765_POWER_H
> > +#define _DT_BINDINGS_POWER_MT6765_POWER_H
> > +
> > +#define MT6765_POWER_DOMAIN_CONN 0
> > +#define MT6765_POWER_DOMAIN_MM 1
> > +#define MT6765_POWER_DOMAIN_MFG_ASYNC 2
> > +#define MT6765_POWER_DOMAIN_ISP 3
> > +#define MT6765_POWER_DOMAIN_MFG 4
> > +#define MT6765_POWER_DOMAIN_MFG_CORE0 5
> > +#define MT6765_POWER_DOMAIN_CAM 6
> > +#define MT6765_POWER_DOMAIN_VCODEC 7
> > +
> > +#endif /* _DT_BINDINGS_POWER_MT6765_POWER_H */
>
>
^ permalink raw reply
* Re: [PATCH v5 05/11] clk: mediatek: add mt6765 clock IDs
From: Mars Cheng @ 2018-07-18 4:23 UTC (permalink / raw)
To: Matthias Brugger
Cc: Rob Herring, Marc Zyngier, Ryder Lee, Stephen Boyd, Sean Wang,
CC Hwang, Loda Chou, linux-kernel, linux-mediatek, devicetree,
wsd_upstream, linux-serial, linux-arm-kernel, linux-clk,
Owen Chen
In-Reply-To: <a7ede0bf-e905-bf37-7e21-60226f56b9ab@gmail.com>
Hi Matthias
On Tue, 2018-07-17 at 12:24 +0200, Matthias Brugger wrote:
>
> On 17/07/18 10:52, Mars Cheng wrote:
> > Signed-off-by: Mars Cheng <mars.cheng@mediatek.com>
> > Signed-off-by: Owen Chen <owen.chen@mediatek.com>
>
> Please provide a commit message.
>
> Thanks,
> Matthias
Got it, it is my bad, will add it.
Thanks.
>
> > ---
> > include/dt-bindings/clock/mt6765-clk.h | 313 ++++++++++++++++++++++++++++++++
> > 1 file changed, 313 insertions(+)
> > create mode 100644 include/dt-bindings/clock/mt6765-clk.h
> >
> > diff --git a/include/dt-bindings/clock/mt6765-clk.h b/include/dt-bindings/clock/mt6765-clk.h
> > new file mode 100644
> > index 0000000..eb97e56
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/mt6765-clk.h
> > @@ -0,0 +1,313 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _DT_BINDINGS_CLK_MT6765_H
> > +#define _DT_BINDINGS_CLK_MT6765_H
> > +
> > +/* FIX Clks */
> > +#define CLK_TOP_CLK26M 0
> > +
> > +/* APMIXEDSYS */
> > +#define CLK_APMIXED_ARMPLL_L 0
> > +#define CLK_APMIXED_ARMPLL 1
> > +#define CLK_APMIXED_CCIPLL 2
> > +#define CLK_APMIXED_MAINPLL 3
> > +#define CLK_APMIXED_MFGPLL 4
> > +#define CLK_APMIXED_MMPLL 5
> > +#define CLK_APMIXED_UNIV2PLL 6
> > +#define CLK_APMIXED_MSDCPLL 7
> > +#define CLK_APMIXED_APLL1 8
> > +#define CLK_APMIXED_MPLL 9
> > +#define CLK_APMIXED_ULPOSC1 10
> > +#define CLK_APMIXED_ULPOSC2 11
> > +#define CLK_APMIXED_SSUSB26M 12
> > +#define CLK_APMIXED_APPLL26M 13
> > +#define CLK_APMIXED_MIPIC0_26M 14
> > +#define CLK_APMIXED_MDPLLGP26M 15
> > +#define CLK_APMIXED_MMSYS_F26M 16
> > +#define CLK_APMIXED_UFS26M 17
> > +#define CLK_APMIXED_MIPIC1_26M 18
> > +#define CLK_APMIXED_MEMPLL26M 19
> > +#define CLK_APMIXED_CLKSQ_LVPLL_26M 20
> > +#define CLK_APMIXED_MIPID0_26M 21
> > +#define CLK_APMIXED_NR_CLK 22
> > +
> > +/* TOPCKGEN */
> > +#define CLK_TOP_SYSPLL 0
> > +#define CLK_TOP_SYSPLL_D2 1
> > +#define CLK_TOP_SYSPLL1_D2 2
> > +#define CLK_TOP_SYSPLL1_D4 3
> > +#define CLK_TOP_SYSPLL1_D8 4
> > +#define CLK_TOP_SYSPLL1_D16 5
> > +#define CLK_TOP_SYSPLL_D3 6
> > +#define CLK_TOP_SYSPLL2_D2 7
> > +#define CLK_TOP_SYSPLL2_D4 8
> > +#define CLK_TOP_SYSPLL2_D8 9
> > +#define CLK_TOP_SYSPLL_D5 10
> > +#define CLK_TOP_SYSPLL3_D2 11
> > +#define CLK_TOP_SYSPLL3_D4 12
> > +#define CLK_TOP_SYSPLL_D7 13
> > +#define CLK_TOP_SYSPLL4_D2 14
> > +#define CLK_TOP_SYSPLL4_D4 15
> > +#define CLK_TOP_USB20_192M 16
> > +#define CLK_TOP_USB20_192M_D4 17
> > +#define CLK_TOP_USB20_192M_D8 18
> > +#define CLK_TOP_USB20_192M_D16 19
> > +#define CLK_TOP_USB20_192M_D32 20
> > +#define CLK_TOP_UNIVPLL 21
> > +#define CLK_TOP_UNIVPLL_D2 22
> > +#define CLK_TOP_UNIVPLL1_D2 23
> > +#define CLK_TOP_UNIVPLL1_D4 24
> > +#define CLK_TOP_UNIVPLL_D3 25
> > +#define CLK_TOP_UNIVPLL2_D2 26
> > +#define CLK_TOP_UNIVPLL2_D4 27
> > +#define CLK_TOP_UNIVPLL2_D8 28
> > +#define CLK_TOP_UNIVPLL2_D32 29
> > +#define CLK_TOP_UNIVPLL_D5 30
> > +#define CLK_TOP_UNIVPLL3_D2 31
> > +#define CLK_TOP_UNIVPLL3_D4 32
> > +#define CLK_TOP_MMPLL 33
> > +#define CLK_TOP_MMPLL_D2 34
> > +#define CLK_TOP_MPLL 35
> > +#define CLK_TOP_DA_MPLL_104M_DIV 36
> > +#define CLK_TOP_DA_MPLL_52M_DIV 37
> > +#define CLK_TOP_MFGPLL 38
> > +#define CLK_TOP_MSDCPLL 39
> > +#define CLK_TOP_MSDCPLL_D2 40
> > +#define CLK_TOP_APLL1 41
> > +#define CLK_TOP_APLL1_D2 42
> > +#define CLK_TOP_APLL1_D4 43
> > +#define CLK_TOP_APLL1_D8 44
> > +#define CLK_TOP_ULPOSC1 45
> > +#define CLK_TOP_ULPOSC1_D2 46
> > +#define CLK_TOP_ULPOSC1_D4 47
> > +#define CLK_TOP_ULPOSC1_D8 48
> > +#define CLK_TOP_ULPOSC1_D16 49
> > +#define CLK_TOP_ULPOSC1_D32 50
> > +#define CLK_TOP_DMPLL 51
> > +#define CLK_TOP_F_FRTC 52
> > +#define CLK_TOP_F_F26M 53
> > +#define CLK_TOP_AXI 54
> > +#define CLK_TOP_MM 55
> > +#define CLK_TOP_SCP 56
> > +#define CLK_TOP_MFG 57
> > +#define CLK_TOP_F_FUART 58
> > +#define CLK_TOP_SPI 59
> > +#define CLK_TOP_MSDC50_0 60
> > +#define CLK_TOP_MSDC30_1 61
> > +#define CLK_TOP_AUDIO 62
> > +#define CLK_TOP_AUD_1 63
> > +#define CLK_TOP_AUD_ENGEN1 64
> > +#define CLK_TOP_F_FDISP_PWM 65
> > +#define CLK_TOP_SSPM 66
> > +#define CLK_TOP_DXCC 67
> > +#define CLK_TOP_I2C 68
> > +#define CLK_TOP_F_FPWM 69
> > +#define CLK_TOP_F_FSENINF 70
> > +#define CLK_TOP_AES_FDE 71
> > +#define CLK_TOP_F_BIST2FPC 72
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL0 73
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL1 74
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL2 75
> > +#define CLK_TOP_DA_USB20_48M_DIV 76
> > +#define CLK_TOP_DA_UNIV_48M_DIV 77
> > +#define CLK_TOP_APLL12_DIV0 78
> > +#define CLK_TOP_APLL12_DIV1 79
> > +#define CLK_TOP_APLL12_DIV2 80
> > +#define CLK_TOP_APLL12_DIV3 81
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL0_EN 82
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL1_EN 83
> > +#define CLK_TOP_ARMPLL_DIVIDER_PLL2_EN 84
> > +#define CLK_TOP_FMEM_OCC_DRC_EN 85
> > +#define CLK_TOP_USB20_48M_EN 86
> > +#define CLK_TOP_UNIVPLL_48M_EN 87
> > +#define CLK_TOP_MPLL_104M_EN 88
> > +#define CLK_TOP_MPLL_52M_EN 89
> > +#define CLK_TOP_F_UFS_MP_SAP_CFG_EN 90
> > +#define CLK_TOP_F_BIST2FPC_EN 91
> > +#define CLK_TOP_MD_32K 92
> > +#define CLK_TOP_MD_26M 93
> > +#define CLK_TOP_MD2_32K 94
> > +#define CLK_TOP_MD2_26M 95
> > +#define CLK_TOP_AXI_SEL 96
> > +#define CLK_TOP_MEM_SEL 97
> > +#define CLK_TOP_MM_SEL 98
> > +#define CLK_TOP_SCP_SEL 99
> > +#define CLK_TOP_MFG_SEL 100
> > +#define CLK_TOP_ATB_SEL 101
> > +#define CLK_TOP_CAMTG_SEL 102
> > +#define CLK_TOP_CAMTG1_SEL 103
> > +#define CLK_TOP_CAMTG2_SEL 104
> > +#define CLK_TOP_CAMTG3_SEL 105
> > +#define CLK_TOP_UART_SEL 106
> > +#define CLK_TOP_SPI_SEL 107
> > +#define CLK_TOP_MSDC50_0_HCLK_SEL 108
> > +#define CLK_TOP_MSDC50_0_SEL 109
> > +#define CLK_TOP_MSDC30_1_SEL 110
> > +#define CLK_TOP_AUDIO_SEL 111
> > +#define CLK_TOP_AUD_INTBUS_SEL 112
> > +#define CLK_TOP_AUD_1_SEL 113
> > +#define CLK_TOP_AUD_ENGEN1_SEL 114
> > +#define CLK_TOP_DISP_PWM_SEL 115
> > +#define CLK_TOP_SSPM_SEL 116
> > +#define CLK_TOP_DXCC_SEL 117
> > +#define CLK_TOP_USB_TOP_SEL 118
> > +#define CLK_TOP_SPM_SEL 119
> > +#define CLK_TOP_I2C_SEL 120
> > +#define CLK_TOP_PWM_SEL 121
> > +#define CLK_TOP_SENINF_SEL 122
> > +#define CLK_TOP_AES_FDE_SEL 123
> > +#define CLK_TOP_PWRAP_ULPOSC_SEL 124
> > +#define CLK_TOP_CAMTM_SEL 125
> > +#define CLK_TOP_NR_CLK 126
> > +
> > +/* INFRACFG */
> > +#define CLK_IFR_ICUSB 0
> > +#define CLK_IFR_GCE 1
> > +#define CLK_IFR_THERM 2
> > +#define CLK_IFR_I2C_AP 3
> > +#define CLK_IFR_I2C_CCU 4
> > +#define CLK_IFR_I2C_SSPM 5
> > +#define CLK_IFR_I2C_RSV 6
> > +#define CLK_IFR_PWM_HCLK 7
> > +#define CLK_IFR_PWM1 8
> > +#define CLK_IFR_PWM2 9
> > +#define CLK_IFR_PWM3 10
> > +#define CLK_IFR_PWM4 11
> > +#define CLK_IFR_PWM5 12
> > +#define CLK_IFR_PWM 13
> > +#define CLK_IFR_UART0 14
> > +#define CLK_IFR_UART1 15
> > +#define CLK_IFR_GCE_26M 16
> > +#define CLK_IFR_CQ_DMA_FPC 17
> > +#define CLK_IFR_BTIF 18
> > +#define CLK_IFR_SPI0 19
> > +#define CLK_IFR_MSDC0 20
> > +#define CLK_IFR_MSDC1 21
> > +#define CLK_IFR_TRNG 22
> > +#define CLK_IFR_AUXADC 23
> > +#define CLK_IFR_CCIF1_AP 24
> > +#define CLK_IFR_CCIF1_MD 25
> > +#define CLK_IFR_AUXADC_MD 26
> > +#define CLK_IFR_AP_DMA 27
> > +#define CLK_IFR_DEVICE_APC 28
> > +#define CLK_IFR_CCIF_AP 29
> > +#define CLK_IFR_AUDIO 30
> > +#define CLK_IFR_CCIF_MD 31
> > +#define CLK_IFR_RG_PWM_FBCLK6 32
> > +#define CLK_IFR_DISP_PWM 33
> > +#define CLK_IFR_CLDMA_BCLK 34
> > +#define CLK_IFR_AUDIO_26M_BCLK 35
> > +#define CLK_IFR_SPI1 36
> > +#define CLK_IFR_I2C4 37
> > +#define CLK_IFR_SPI2 38
> > +#define CLK_IFR_SPI3 39
> > +#define CLK_IFR_I2C5 40
> > +#define CLK_IFR_I2C5_ARBITER 41
> > +#define CLK_IFR_I2C5_IMM 42
> > +#define CLK_IFR_I2C1_ARBITER 43
> > +#define CLK_IFR_I2C1_IMM 44
> > +#define CLK_IFR_I2C2_ARBITER 45
> > +#define CLK_IFR_I2C2_IMM 46
> > +#define CLK_IFR_SPI4 47
> > +#define CLK_IFR_SPI5 48
> > +#define CLK_IFR_CQ_DMA 49
> > +#define CLK_IFR_FAES_FDE 50
> > +#define CLK_IFR_MSDC0_SELF 51
> > +#define CLK_IFR_MSDC1_SELF 52
> > +#define CLK_IFR_I2C6 53
> > +#define CLK_IFR_AP_MSDC0 54
> > +#define CLK_IFR_MD_MSDC0 55
> > +#define CLK_IFR_MSDC0_SRC 56
> > +#define CLK_IFR_MSDC1_SRC 57
> > +#define CLK_IFR_AES_TOP0_BCLK 58
> > +#define CLK_IFR_MCU_PM_BCLK 59
> > +#define CLK_IFR_CCIF2_AP 60
> > +#define CLK_IFR_CCIF2_MD 61
> > +#define CLK_IFR_CCIF3_AP 62
> > +#define CLK_IFR_CCIF3_MD 63
> > +#define CLK_IFR_NR_CLK 64
> > +
> > +/* AUDIO */
> > +#define CLK_AUDIO_AFE 0
> > +#define CLK_AUDIO_22M 1
> > +#define CLK_AUDIO_APLL_TUNER 2
> > +#define CLK_AUDIO_ADC 3
> > +#define CLK_AUDIO_DAC 4
> > +#define CLK_AUDIO_DAC_PREDIS 5
> > +#define CLK_AUDIO_TML 6
> > +#define CLK_AUDIO_I2S1_BCLK 7
> > +#define CLK_AUDIO_I2S2_BCLK 8
> > +#define CLK_AUDIO_I2S3_BCLK 9
> > +#define CLK_AUDIO_I2S4_BCLK 10
> > +#define CLK_AUDIO_NR_CLK 11
> > +
> > +/* MIPI_RX_ANA_CSI0A */
> > +
> > +#define CLK_MIPI0A_CSR_CSI_EN_0A 0
> > +#define CLK_MIPI0A_NR_CLK 1
> > +
> > +/* MMSYS_CONFIG */
> > +
> > +#define CLK_MM_MDP_RDMA0 0
> > +#define CLK_MM_MDP_CCORR0 1
> > +#define CLK_MM_MDP_RSZ0 2
> > +#define CLK_MM_MDP_RSZ1 3
> > +#define CLK_MM_MDP_TDSHP0 4
> > +#define CLK_MM_MDP_WROT0 5
> > +#define CLK_MM_MDP_WDMA0 6
> > +#define CLK_MM_DISP_OVL0 7
> > +#define CLK_MM_DISP_OVL0_2L 8
> > +#define CLK_MM_DISP_RSZ0 9
> > +#define CLK_MM_DISP_RDMA0 10
> > +#define CLK_MM_DISP_WDMA0 11
> > +#define CLK_MM_DISP_COLOR0 12
> > +#define CLK_MM_DISP_CCORR0 13
> > +#define CLK_MM_DISP_AAL0 14
> > +#define CLK_MM_DISP_GAMMA0 15
> > +#define CLK_MM_DISP_DITHER0 16
> > +#define CLK_MM_DSI0 17
> > +#define CLK_MM_FAKE_ENG 18
> > +#define CLK_MM_SMI_COMMON 19
> > +#define CLK_MM_SMI_LARB0 20
> > +#define CLK_MM_SMI_COMM0 21
> > +#define CLK_MM_SMI_COMM1 22
> > +#define CLK_MM_CAM_MDP 23
> > +#define CLK_MM_SMI_IMG 24
> > +#define CLK_MM_SMI_CAM 25
> > +#define CLK_MM_IMG_DL_RELAY 26
> > +#define CLK_MM_IMG_DL_ASYNC_TOP 27
> > +#define CLK_MM_DIG_DSI 28
> > +#define CLK_MM_F26M_HRTWT 29
> > +#define CLK_MM_NR_CLK 30
> > +
> > +/* IMGSYS */
> > +
> > +#define CLK_IMG_LARB2 0
> > +#define CLK_IMG_DIP 1
> > +#define CLK_IMG_FDVT 2
> > +#define CLK_IMG_DPE 3
> > +#define CLK_IMG_RSC 4
> > +#define CLK_IMG_NR_CLK 5
> > +
> > +/* VENCSYS */
> > +
> > +#define CLK_VENC_SET0_LARB 0
> > +#define CLK_VENC_SET1_VENC 1
> > +#define CLK_VENC_SET2_JPGENC 2
> > +#define CLK_VENC_SET3_VDEC 3
> > +#define CLK_VENC_NR_CLK 4
> > +
> > +/* CAMSYS */
> > +
> > +#define CLK_CAM_LARB3 0
> > +#define CLK_CAM_DFP_VAD 1
> > +#define CLK_CAM 2
> > +#define CLK_CAMTG 3
> > +#define CLK_CAM_SENINF 4
> > +#define CLK_CAMSV0 5
> > +#define CLK_CAMSV1 6
> > +#define CLK_CAMSV2 7
> > +#define CLK_CAM_CCU 8
> > +#define CLK_CAM_NR_CLK 9
> > +
> > +#endif /* _DT_BINDINGS_CLK_MT6765_H */
> >
^ 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