Re: [PATCH v2 02/13] spi: Replace all spi->chip_select and spi->cs_gpiod references with function call
On Fri, Jan 20, 2023 at 12:23:31AM +0530, Amit Kumar Mahapatra wrote:
> Supporting multi-cs in spi drivers would require the chip_select & cs_gpiod
> members of struct spi_device to be an array. But changing the type of these
> members to array would break the spi driver functionality. To make the
> transition smoother introduced four new APIs to get/set the
> spi->chip_select & spi->cs_gpiod and replaced all spi->chip_select and
> spi->cs_gpiod references with get or set API calls.
> While adding multi-cs support in further patches the chip_select & cs_gpiod
> members of the spi_device structure would be converted to arrays & the
> "idx" parameter of the APIs would be used as array index i.e.,
> spi->chip_select[idx] & spi->cs_gpiod[idx] respectively.
>
> Signed-off-by: Amit Kumar Mahapatra
> ---
[nip]
> drivers/spi/spi-dw-core.c | 2 +-
> drivers/spi/spi-dw-mmio.c | 4 ++--
[nip]
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 99edddf9958b..4fd1aa800cc3 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -103,7 +103,7 @@ void dw_spi_set_cs(struct spi_device *spi, bool enable)
>* support active-high or active-low CS level.
>*/
> if (cs_high == enable)
> - dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
> + dw_writel(dws, DW_SPI_SER, BIT(spi_get_chipselect(spi, 0)));
> else
> dw_writel(dws, DW_SPI_SER, 0);
> }
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 26c40ea6dd12..d511da766ce8 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -65,7 +65,7 @@ static void dw_spi_mscc_set_cs(struct spi_device *spi, bool
> enable)
> struct dw_spi *dws = spi_master_get_devdata(spi->master);
> struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio,
> dws);
> struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
> - u32 cs = spi->chip_select;
> + u32 cs = spi_get_chipselect(spi, 0);
>
> if (cs < 4) {
> u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
> @@ -138,7 +138,7 @@ static void dw_spi_sparx5_set_cs(struct spi_device *spi,
> bool enable)
> struct dw_spi *dws = spi_master_get_devdata(spi->master);
> struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio,
> dws);
> struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
> - u8 cs = spi->chip_select;
> + u8 cs = spi_get_chipselect(spi, 0);
>
> if (!enable) {
> /* CS override drive enable */
For the DW SSI part:
Reviewed-by: Serge Semin
-Serge(y)
[nip]
Re: [PATCH v3 05/15] dt-bindings: dma: dw-axi-dmac: extend the number of interrupts
On Wed, Jun 29, 2022 at 07:43:34PM +0100, Conor Dooley wrote: > From: Conor Dooley > > The Canaan k210 apparently has a Sysnopsys Designware AXI DMA > controller, but according to the documentation & devicetree it has 6 > interrupts rather than the standard one. Support the 6 interrupt > configuration by unconditionally extending the binding to a maximum of > 8 per-channel interrupts thereby matching the number of possible > channels. > > Link: > https://canaan-creative.com/wp-content/uploads/2020/03/kendryte_standalone_programming_guide_20190311144158_en.pdf > #Page 51 > Signed-off-by: Conor Dooley > --- > Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml > b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml > index 4324a94b26b2..e33ef22aec9c 100644 > --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml > +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml > @@ -34,7 +34,9 @@ properties: >- const: axidma_apb_regs > >interrupts: > -maxItems: 1 > +description: per channel interrupts Description is inaccurate. It's either combined or per-channel IRQs. Other than that: Reviewed-by: Serge Semin -Sergey > +minItems: 1 > +maxItems: 8 > >clocks: > items: > -- > 2.36.1 >
Re: [PATCH v3 08/15] riscv: dts: canaan: fix the k210's timer nodes
On Wed, Jun 29, 2022 at 07:43:37PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> The timers on the k210 have non standard interrupt configurations,
> which leads to dtbs_check warnings:
>
> k210_generic.dtb: timer@502d: interrupts: [[14], [15]] is too long
> From schema: Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
>
> Split the timer nodes in two, so that the second timer in the IP block
> can actually be accessed & in the process solve the dtbs_check warning.
Reviewed-by: Serge Semin
Just to note. IMO the DW APB Timer driver has been incorrectly
designed in the first place. The dts-node is supposed to describe the
whole IP-core timers set as the original Canaan k210 DT-file expected,
since there are common CSRs in the registers range, which currently
get to be unreachable. But since the DT-bindings has already been
defined that way in the framework of DW APB Timer driver alas there
is nothing we can do to fix it.
-Sergey
>
> Signed-off-by: Conor Dooley
> ---
> arch/riscv/boot/dts/canaan/k210.dtsi | 46 +++-
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi
> b/arch/riscv/boot/dts/canaan/k210.dtsi
> index cd4eae82d8b2..72f70128d751 100644
> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
> @@ -319,28 +319,58 @@ fpioa: pinmux@502b {
>
> timer0: timer@502d {
> compatible = "snps,dw-apb-timer";
> - reg = <0x502D 0x100>;
> - interrupts = <14>, <15>;
> + reg = <0x502D 0x14>;
> + interrupts = <14>;
> clocks = < K210_CLK_TIMER0>,
>< K210_CLK_APB0>;
> clock-names = "timer", "pclk";
> resets = < K210_RST_TIMER0>;
> };
>
> - timer1: timer@502e {
> + timer1: timer@502d0014 {
> compatible = "snps,dw-apb-timer";
> - reg = <0x502E 0x100>;
> - interrupts = <16>, <17>;
> + reg = <0x502D0014 0x14>;
> + interrupts = <15>;
> + clocks = < K210_CLK_TIMER0>,
> + < K210_CLK_APB0>;
> + clock-names = "timer", "pclk";
> + resets = < K210_RST_TIMER0>;
> + };
> +
> + timer2: timer@502e {
> + compatible = "snps,dw-apb-timer";
> + reg = <0x502E 0x14>;
> + interrupts = <16>;
> clocks = < K210_CLK_TIMER1>,
>< K210_CLK_APB0>;
> clock-names = "timer", "pclk";
> resets = < K210_RST_TIMER1>;
> };
>
> - timer2: timer@502f {
> + timer3: timer@502e0014 {
> + compatible = "snps,dw-apb-timer";
> + reg = <0x502E0014 0x114>;
> + interrupts = <17>;
> + clocks = < K210_CLK_TIMER1>,
> + < K210_CLK_APB0>;
> + clock-names = "timer", "pclk";
> + resets = < K210_RST_TIMER1>;
> + };
> +
> + timer4: timer@502f {
> + compatible = "snps,dw-apb-timer";
> + reg = <0x502F 0x14>;
> + interrupts = <18>;
> + clocks = < K210_CLK_TIMER2>,
> + < K210_CLK_APB0>;
> + clock-names = "timer", "pclk";
> + resets = < K210_RST_TIMER2>;
> + };
> +
> + timer5: timer@502f0014 {
> compatible = "snps,dw-apb-timer";
> - reg = <0x502F 0x100>;
> - interrupts = <18>, <19>;
> + reg = <0x502F0014 0x14>;
> + interrupts = <19>;
> clocks = < K210_CLK_TIMER2>,
>< K210_CLK_APB0>;
> clock-names = "timer", "pclk";
> --
> 2.36.1
>
Re: [PATCH v3 04/15] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width
On Wed, Jun 29, 2022 at 07:43:33PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> Most users of dw-apb-ssi use spi-{r,t}x-bus-width of 1, however the
> Canaan k210 is wired up for a width of 4.
> Quoting Serge:
> The modern DW APB SSI controllers of v.4.* and newer also support the
> enhanced SPI Modes too (Dual, Quad and Octal). Since the IP-core
> version is auto-detected at run-time there is no way to create a
> DT-schema correctly constraining the Rx/Tx SPI bus widths.
> /endquote
>
> As such, drop the restriction on only supporting a bus width of 1.
Reviewed-by: Serge Semin
-Sergey
>
> Link:
> https://lore.kernel.org/all/20220620205654.g7fyipwytbww5757@mobilestation/
> Signed-off-by: Conor Dooley
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index e25d44c218f2..0a43d6e0ef91 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -143,12 +143,6 @@ patternProperties:
> minimum: 0
> maximum: 3
>
> - spi-rx-bus-width:
> -const: 1
> -
> - spi-tx-bus-width:
> -const: 1
> -
> unevaluatedProperties: false
>
> required:
> --
> 2.36.1
>
Re: [PATCH 05/14] dt-bindings: timer: add Canaan k210 to Synopsys DesignWare timer
Hi Rob,
On Mon, Jun 27, 2022 at 05:30:25PM -0600, Rob Herring wrote:
> On Sat, Jun 18, 2022 at 01:30:27PM +0100, Conor Dooley wrote:
> > From: Conor Dooley
> >
> > The Canaan k210 apparently has a Sysnopsys Designware timer but
> > according to the documentation & devicetree it has 2 interrupts rather
> > than the standard one. Add a custom compatible that supports the 2
> > interrupt configuration and falls back to the standard binding (which
> > is currently the one in use in the devicetree entry).
> >
> > Link:
> > https://canaan-creative.com/wp-content/uploads/2020/03/kendryte_standalone_programming_guide_20190311144158_en.pdf
> > #Page 58
> > Signed-off-by: Conor Dooley
> > ---
> > .../bindings/timer/snps,dw-apb-timer.yaml | 28 +++
> > 1 file changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> > b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> > index d33c9205a909..9a76acc7a66f 100644
> > --- a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> > +++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> > @@ -12,6 +12,9 @@ maintainers:
> > properties:
> >compatible:
> > oneOf:
> > + - items:
> > + - const: canaan,k210-apb-timer
> > + - const: snps,dw-apb-timer
> >- const: snps,dw-apb-timer
> >- enum:
> >- snps,dw-apb-timer-sp
> > @@ -21,9 +24,6 @@ properties:
> >reg:
> > maxItems: 1
> >
> > - interrupts:
> > -maxItems: 1
> > -
> >resets:
> > maxItems: 1
> >
> > @@ -41,7 +41,23 @@ properties:
> >
> >clock-frequency: true
> >
> > -additionalProperties: false
> > +unevaluatedProperties: false
> > +
> > +if:
> > + properties:
> > +compatible:
> > + contains:
> > +const: canaan,k210-apb-timer
> > +
> > +then:
> > + properties:
> > +interrupts:
> > + maxItems: 2
>
> When more than 1, you need to define what they are and the order.
Please see my note to v2 of this patch:
Link:
https://lore.kernel.org/lkml/20220627211314.dc2hempelyl5ayjg@mobilestation/
-Sergey
>
> > +
> > +else:
> > + properties:
> > +interrupts:
> > + maxItems: 1
> >
> > required:
> >- compatible
> > @@ -60,8 +76,8 @@ oneOf:
> > examples:
> >- |
> > timer@ffe0 {
> > - compatible = "snps,dw-apb-timer";
> > - interrupts = <0 170 4>;
> > + compatible = "canaan,k210-apb-timer", "snps,dw-apb-timer";
> > + interrupts = <0 170 4>, <0 170 4>;
> >reg = <0xffe0 0x1000>;
> >clocks = <_clk>, <_pclk>;
> >clock-names = "timer", "pclk";
> > --
> > 2.36.1
> >
> >
Re: [PATCH 04/14] dt-bindings: dma: add Canaan k210 to Synopsys DesignWare DMA
On Tue, Jun 28, 2022 at 09:08:32AM +0200, Geert Uytterhoeven wrote:
> Hi Conor,
>
> On Tue, Jun 28, 2022 at 8:30 AM wrote:
> > On 28/06/2022 00:29, Rob Herring wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > >
> > > On Sat, Jun 18, 2022 at 01:30:26PM +0100, Conor Dooley wrote:
> > >> From: Conor Dooley
> > >>
> > >> The Canaan k210 apparently has a Sysnopsys Designware AXI DMA
> > >> controller, but according to the documentation & devicetree it has 6
> > >> interrupts rather than the standard one. Add a custom compatible that
> > >> supports the 6 interrupt configuration which falls back to the standard
> > >> binding which is currently the one in use in the devicetree entry.
> > >
> > > But it works with only 1 interrupt?
> >
> > Seemingly...
>
> The Linux driver uses the only first interrupt.
Folks, there v2 of this series. A more suitable and justified solution has
been given there:
Link:
https://lore.kernel.org/lkml/20220627213450.ukqai4bsjzcwhnlp@mobilestation/
-Sergey
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Re: [PATCH v2 05/16] dt-bindings: dma: add Canaan k210 to Synopsys DesignWare DMA
On Mon, Jun 27, 2022 at 08:39:53PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> The Canaan k210 apparently has a Sysnopsys Designware AXI DMA
> controller, but according to the documentation & devicetree it has 6
> interrupts rather than the standard one. Add a custom compatible that
> supports the 6 interrupt configuration which falls back to the standard
> binding which is currently the one in use in the devicetree entry.
>
> Link:
> https://canaan-creative.com/wp-content/uploads/2020/03/kendryte_standalone_programming_guide_20190311144158_en.pdf
> #Page 58
I believe what you've got here is the per-channel IRQs, which is
activated for the DW AXI DMAC by setting IP-core synthesize parameter
DMAX_INTR_IO_TYPE=1 (CHANNEL AND COMMONREG). That's why there are six
IRQ signals and six DMA-channels available.
Seeing such setting isn't the Canaan k210 specific, but is the DW AXI
DMA controller common property, what would be right in this case is to
unconditionally extend the number of IRQs in the DT-bindings schema.
Please note the DW AXI DMAC driver currently doesn't support the
per-channel IRQs. So most likely the DMA will only work with using the
channel 0 only.
-Sergey
> Signed-off-by: Conor Dooley
> ---
> .../bindings/dma/snps,dw-axi-dmac.yaml| 35 ++-
> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> index 4324a94b26b2..bc85598151ef 100644
> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> @@ -18,9 +18,13 @@ allOf:
>
> properties:
>compatible:
> -enum:
> - - snps,axi-dma-1.01a
> - - intel,kmb-axi-dma
> +oneOf:
> + - items:
> + - const: canaan,k210-axi-dma
> + - const: snps,axi-dma-1.01a
> + - enum:
> + - snps,axi-dma-1.01a
> + - intel,kmb-axi-dma
>
>reg:
> minItems: 1
> @@ -33,9 +37,6 @@ properties:
>- const: axidma_ctrl_regs
>- const: axidma_apb_regs
>
> - interrupts:
> -maxItems: 1
> -
>clocks:
> items:
>- description: Bus Clock
> @@ -92,6 +93,22 @@ properties:
> minimum: 1
> maximum: 256
>
> +if:
> + properties:
> +compatible:
> + contains:
> +const: canaan,k210-axi-dma
> +
> +then:
> + properties:
> +interrupts:
> + maxItems: 6
> +
> +else:
> + properties:
> +interrupts:
> + maxItems: 1
> +
> required:
>- compatible
>- reg
> @@ -105,7 +122,7 @@ required:
>- snps,priority
>- snps,block-size
>
> -additionalProperties: false
> +unevaluatedProperties: false
>
> examples:
>- |
> @@ -113,12 +130,12 @@ examples:
> #include
> /* example with snps,dw-axi-dmac */
> dmac: dma-controller@8 {
> - compatible = "snps,axi-dma-1.01a";
> + compatible = "canaan,k210-axi-dma", "snps,axi-dma-1.01a";
> reg = <0x8 0x400>;
> clocks = <_clk>, <_clk>;
> clock-names = "core-clk", "cfgr-clk";
> interrupt-parent = <>;
> - interrupts = <27>;
> + interrupts = <27>, <28>, <29>, <30>, <31>, <32>;
> #dma-cells = <1>;
> dma-channels = <4>;
> snps,dma-masters = <2>;
> --
> 2.36.1
>
Re: [PATCH v2 06/16] dt-bindings: timer: add Canaan k210 to Synopsys DesignWare timer
On Mon, Jun 27, 2022 at 08:39:54PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> The Canaan k210 apparently has a Sysnopsys Designware timer but
> according to the documentation & devicetree it has 2 interrupts rather
> than the standard one. Add a custom compatible that supports the 2
> interrupt configuration and falls back to the standard binding (which
> is currently the one in use in the devicetree entry).
>
> Link:
> https://canaan-creative.com/wp-content/uploads/2020/03/kendryte_standalone_programming_guide_20190311144158_en.pdf
> #Page 58
Firstly, it's page 51 in the framework of the document pages
enumeration.
Judging by the comment in the document above and what the HW reference
manual says regarding the IRQ signals, what you really have on K210 is
the DW APB Timer IP-cores each configured with two embedded timers.
It's done by the IP-core synthesize parameter NUM_TIMERS={1..8}, which
in your case equals to 2. A similar situation is on our SoC and, for
instance, here:
arch/arm/boot/dts/berlin2q.dtsi
arch/arm/boot/dts/berlin2.dtsi
arch/arm/boot/dts/berlin2cd.dtsi
(Though the Berlin2 APB Timer have been configured with 8 timers.)
So the correct modification would be:
1. Split up the nodes into two ones with one IRQ per each node.
2. Make sure I was right by testing the new dts out.
3. Update the DT-node only and leave the DT-bindings as is.
-Sergey
> Signed-off-by: Conor Dooley
> ---
> .../bindings/timer/snps,dw-apb-timer.yaml | 28 +++
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> index d33c9205a909..9a76acc7a66f 100644
> --- a/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> +++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
> @@ -12,6 +12,9 @@ maintainers:
> properties:
>compatible:
> oneOf:
> + - items:
> + - const: canaan,k210-apb-timer
> + - const: snps,dw-apb-timer
>- const: snps,dw-apb-timer
>- enum:
>- snps,dw-apb-timer-sp
> @@ -21,9 +24,6 @@ properties:
>reg:
> maxItems: 1
>
> - interrupts:
> -maxItems: 1
> -
>resets:
> maxItems: 1
>
> @@ -41,7 +41,23 @@ properties:
>
>clock-frequency: true
>
> -additionalProperties: false
> +unevaluatedProperties: false
> +
> +if:
> + properties:
> +compatible:
> + contains:
> +const: canaan,k210-apb-timer
> +
> +then:
> + properties:
> +interrupts:
> + maxItems: 2
> +
> +else:
> + properties:
> +interrupts:
> + maxItems: 1
>
> required:
>- compatible
> @@ -60,8 +76,8 @@ oneOf:
> examples:
>- |
> timer@ffe0 {
> - compatible = "snps,dw-apb-timer";
> - interrupts = <0 170 4>;
> + compatible = "canaan,k210-apb-timer", "snps,dw-apb-timer";
> + interrupts = <0 170 4>, <0 170 4>;
>reg = <0xffe0 0x1000>;
>clocks = <_clk>, <_pclk>;
>clock-names = "timer", "pclk";
> --
> 2.36.1
>
Re: [PATCH v2 04/16] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width
On Mon, Jun 27, 2022 at 08:39:52PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> Most users of dw-apb-ssi use spi-{r,t}x-bus-width of 1, however the
> Canaan k210 is wired up for a width of 4.
> Quoting Serge:
> The modern DW APB SSI controllers of v.4.* and newer also support the
> enhanced SPI Modes too (Dual, Quad and Octal). Since the IP-core
> version is auto-detected at run-time there is no way to create a
> DT-schema correctly constraining the Rx/Tx SPI bus widths.
> /endquote
>
> As such, drop the restriction on only supporting a bus width of 1.
>
> Link:
> https://lore.kernel.org/all/20220620205654.g7fyipwytbww5757@mobilestation/
> Signed-off-by: Conor Dooley
> ---
> Serge, I dropped your R-b when I swapped to the default
> property since it changed the enum.
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index e25d44c218f2..0a43d6e0ef91 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -143,12 +143,6 @@ patternProperties:
> minimum: 0
> maximum: 3
>
> - spi-rx-bus-width:
> -const: 1
> -
> - spi-tx-bus-width:
> -const: 1
> -
My comment was:
> > > You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> >
> > 8 too? sure.
Then Rob said:
> Then no constraints needed because the common definition already has
> this presumably.
IMO preserving the device-specific constraints even if they match the
generic ones has some maintainability benefits. What if you get to
discover a new HW which supports Hexal mode? Then you would have
needed to update the common schema constraints. But that would have
caused permitting the unsupported bus-mode for all the schemas, which
isn't correct. So as I see it the explicit bus-width enumeration would
be ok to have here. But I'll leave it for Rob to make a final
decision.
Rob
> unevaluatedProperties: false
>
> required:
> --
> 2.36.1
>
Re: [PATCH 06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width for dwc-ssi
On Tue, Jun 21, 2022 at 04:06:21PM +, conor.doo...@microchip.com wrote:
> On 21/06/2022 00:17, Damien Le Moal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On 6/21/22 07:49, Conor Dooley wrote:
> >>
> ---8<---
>
> hmm, well I'll leave that up to people that have Canaan hardware!
> >>>
> >>> I will test this series.
> >>>
> >>
> >> Cool, thanks.
> >> I'll try to get a respin out tomorrow w/ the memory node "unfixed".
> >
> > OK. I will test that then :)
>
> Since the memory node hit that dt-schema snag, I have not sent a v2.
> Going to be AFK for a few days, so I dropped the memory node change,
> changed the spi binding & put the series on:
> git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ canaan
>
> If you get a chance to look at it great, if not I'll send a v2 once
> the memory node is figured out.
commit 84df6ca0f277 ("spi: dt-bindings: dw-apb-ssi: update
spi-{r,t}x-bus-width") looks good to me. Feel free to add my ack tag
to v2 of that patch.
-Sergey
>
> Thanks,
> Conor.
>
Re: [PATCH 06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width for dwc-ssi
Hi Geert
On Tue, Jun 21, 2022 at 09:03:25AM +0200, Geert Uytterhoeven wrote:
> Hi Serge,
>
> On Mon, Jun 20, 2022 at 10:56 PM Serge Semin wrote:
> > On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> > > From: Conor Dooley
> > >
[...]
> > > +
> > > +spi-rx-bus-width:
> > > + const: 1
> > > +
> > > +spi-tx-bus-width:
> > > + const: 1
> >
> > You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> > irrespective from the compatible string. The modern DW APB SSI
> > controllers of v.4.* and newer also support the enhanced SPI Modes too
> > (Dual, Quad and Octal). Since the IP-core version is auto-detected at
> > run-time there is no way to create a DT-schema correctly constraining
> > the Rx/Tx SPI bus widths. So let's keep the
> > compatible-string-independent "patternProperties" here but just extend
> > the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
> > properties values.
> >
> > Note the DW APB SSI/AHB SSI driver currently doesn't support the
> > enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
> > bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
> > DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
> > activated by means of the corresponding CSR. So most likely the DW AHB
> > SSI controllers need some specific setups too.
>
> That doesn't matter here, as DT describes hardware, not software
> limitations.
Can't argue with that.) My note regarding the current DW APB SSI
driver was mainly addressed for the Canaan K210 users, since
should the SoC-based board really have the Quad Tx/Rx SPI bus
most likely the interface won't work with the full bandwidth.
So it is a good reason to perform the platform research and if
possible alter the driver accordingly.
-Sergey
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Re: [PATCH 06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width for dwc-ssi
On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> From: Conor Dooley
>
> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
> this.
>
> Signed-off-by: Conor Dooley
> ---
> .../bindings/spi/snps,dw-apb-ssi.yaml | 48 ++-
> 1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index e25d44c218f2..f2b9e3f062cd 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -135,19 +135,41 @@ properties:
>of the designware controller, and the upper limit is also subject to
>controller configuration.
>
> -patternProperties:
> - "^.*@[0-9a-f]+$":
> -type: object
> -properties:
> - reg:
> -minimum: 0
> -maximum: 3
> -
> - spi-rx-bus-width:
> -const: 1
> -
> - spi-tx-bus-width:
> -const: 1
> +if:
> + properties:
> +compatible:
> + contains:
> +const: snps,dwc-ssi-1.01a
> +
> +then:
> + patternProperties:
> +"^.*@[0-9a-f]+$":
> + type: object
> + properties:
> +reg:
> + minimum: 0
> + maximum: 3
> +
> +spi-rx-bus-width:
> + const: 4
> +
> +spi-tx-bus-width:
> + const: 4
> +
> +else:
> + patternProperties:
> +"^.*@[0-9a-f]+$":
> + type: object
> + properties:
> +reg:
> + minimum: 0
> + maximum: 3
> +
> +spi-rx-bus-width:
> + const: 1
> +
> +spi-tx-bus-width:
> + const: 1
You can just use a more relaxed constraint "enum: [1 2 4 8]" here
irrespective from the compatible string. The modern DW APB SSI
controllers of v.4.* and newer also support the enhanced SPI Modes too
(Dual, Quad and Octal). Since the IP-core version is auto-detected at
run-time there is no way to create a DT-schema correctly constraining
the Rx/Tx SPI bus widths. So let's keep the
compatible-string-independent "patternProperties" here but just extend
the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
properties values.
Note the DW APB SSI/AHB SSI driver currently doesn't support the
enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
activated by means of the corresponding CSR. So most likely the DW AHB
SSI controllers need some specific setups too.
-Sergey
>
> unevaluatedProperties: false
>
> --
> 2.36.1
>
Re: [PATCH] drm: Permit video-buffers writecombine mapping for MIPS
Hello Sean,
On Tue, Jun 18, 2019 at 03:52:04PM -0400, Sean Paul wrote:
> On Mon, Jun 17, 2019 at 04:47:30PM +0300, Serge Semin wrote:
> > Hello folks,
> >
> > Any updates of this patch status? It has been here for about two months.
> >
>
> Sorry for the mixup, looks like this one just fell through the cracks. I've
> applied it to drm-misc-next with the attached Ack and Review.
>
Great! Thank you.
Regards,
-Sergey
> Sean
>
>
> > Regards,
> > -Sergey
> >
> > On Tue, Apr 23, 2019 at 03:31:22PM +0300, Serge Semin wrote:
> > > Since commit 4b050ba7a66c ("MIPS: pgtable.h: Implement the
> > > pgprot_writecombine function for MIPS") and commit c4687b15a848 ("MIPS:
> > > Fix
> > > definition of pgprot_writecombine()") write-combine vma mapping is
> > > available to be used by kernel subsystems for MIPS. In particular the
> > > uncached accelerated attribute is requested to be set by ioremap_wc()
> > > method and by generic PCI memory pages/ranges mapping methods. The same
> > > is done by the drm_io_prot()/ttm_io_prot() functions in case if
> > > write-combine flag is set for vma's passed for mapping. But for some
> > > reason the pgprot_writecombine() method calling is ifdefed to be a
> > > platform-specific with MIPS system being marked as lacking of one. At the
> > > very least it doesn't reflect the current MIPS platform implementation.
> > > So in order to improve the DRM subsystem performance on MIPS with UCA
> > > mapping enabled, we need to have pgprot_writecombine() called for buffers,
> > > which need store operations being combined. In case if particular MIPS
> > > chip doesn't support the UCA attribute, the mapping will fall back to
> > > noncached.
> > >
> > > Cc: Ralf Baechle
> > > Cc: Paul Burton
> > > Cc: James Hogan
> > > Signed-off-by: Vadim V. Vlasov
> > > Signed-off-by: Serge Semin
> > > ---
> > > drivers/gpu/drm/drm_vm.c | 5 +++--
> > > drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++--
> > > 2 files changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> > > index c3301046dfaa..50178dc64060 100644
> > > --- a/drivers/gpu/drm/drm_vm.c
> > > +++ b/drivers/gpu/drm/drm_vm.c
> > > @@ -62,7 +62,8 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
> > > /* We don't want graphics memory to be mapped encrypted */
> > > tmp = pgprot_decrypted(tmp);
> > >
> > > -#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__)
> > > +#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) || \
> > > +defined(__mips__)
> > > if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
> > > tmp = pgprot_noncached(tmp);
> > > else
> > > @@ -73,7 +74,7 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
> > > tmp = pgprot_writecombine(tmp);
> > > else
> > > tmp = pgprot_noncached(tmp);
> > > -#elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
> > > +#elif defined(__sparc__) || defined(__arm__)
> > > tmp = pgprot_noncached(tmp);
> > > #endif
> > > return tmp;
> > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > index 895d77d799e4..9f918b992f7e 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > @@ -539,13 +539,13 @@ pgprot_t ttm_io_prot(uint32_t caching_flags,
> > > pgprot_t tmp)
> > > tmp = pgprot_noncached(tmp);
> > > #endif
> > > #if defined(__ia64__) || defined(__arm__) || defined(__aarch64__) || \
> > > -defined(__powerpc__)
> > > +defined(__powerpc__) || defined(__mips__)
> > > if (caching_flags & TTM_PL_FLAG_WC)
> > > tmp = pgprot_writecombine(tmp);
> > > else
> > > tmp = pgprot_noncached(tmp);
> > > #endif
> > > -#if defined(__sparc__) || defined(__mips__)
> > > +#if defined(__sparc__)
> > > tmp = pgprot_noncached(tmp);
> > > #endif
> > > return tmp;
> > > --
> > > 2.21.0
> > >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] drm: Permit video-buffers writecombine mapping for MIPS
Hello folks,
Any updates of this patch status? It has been here for about two months.
Regards,
-Sergey
On Tue, Apr 23, 2019 at 03:31:22PM +0300, Serge Semin wrote:
> Since commit 4b050ba7a66c ("MIPS: pgtable.h: Implement the
> pgprot_writecombine function for MIPS") and commit c4687b15a848 ("MIPS: Fix
> definition of pgprot_writecombine()") write-combine vma mapping is
> available to be used by kernel subsystems for MIPS. In particular the
> uncached accelerated attribute is requested to be set by ioremap_wc()
> method and by generic PCI memory pages/ranges mapping methods. The same
> is done by the drm_io_prot()/ttm_io_prot() functions in case if
> write-combine flag is set for vma's passed for mapping. But for some
> reason the pgprot_writecombine() method calling is ifdefed to be a
> platform-specific with MIPS system being marked as lacking of one. At the
> very least it doesn't reflect the current MIPS platform implementation.
> So in order to improve the DRM subsystem performance on MIPS with UCA
> mapping enabled, we need to have pgprot_writecombine() called for buffers,
> which need store operations being combined. In case if particular MIPS
> chip doesn't support the UCA attribute, the mapping will fall back to
> noncached.
>
> Cc: Ralf Baechle
> Cc: Paul Burton
> Cc: James Hogan
> Signed-off-by: Vadim V. Vlasov
> Signed-off-by: Serge Semin
> ---
> drivers/gpu/drm/drm_vm.c | 5 +++--
> drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index c3301046dfaa..50178dc64060 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -62,7 +62,8 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
> /* We don't want graphics memory to be mapped encrypted */
> tmp = pgprot_decrypted(tmp);
>
> -#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__)
> +#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) || \
> +defined(__mips__)
> if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
> tmp = pgprot_noncached(tmp);
> else
> @@ -73,7 +74,7 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
> tmp = pgprot_writecombine(tmp);
> else
> tmp = pgprot_noncached(tmp);
> -#elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
> +#elif defined(__sparc__) || defined(__arm__)
> tmp = pgprot_noncached(tmp);
> #endif
> return tmp;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 895d77d799e4..9f918b992f7e 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -539,13 +539,13 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t
> tmp)
> tmp = pgprot_noncached(tmp);
> #endif
> #if defined(__ia64__) || defined(__arm__) || defined(__aarch64__) || \
> -defined(__powerpc__)
> +defined(__powerpc__) || defined(__mips__)
> if (caching_flags & TTM_PL_FLAG_WC)
> tmp = pgprot_writecombine(tmp);
> else
> tmp = pgprot_noncached(tmp);
> #endif
> -#if defined(__sparc__) || defined(__mips__)
> +#if defined(__sparc__)
> tmp = pgprot_noncached(tmp);
> #endif
> return tmp;
> --
> 2.21.0
>
[PATCH] drm: Permit video-buffers writecombine mapping for MIPS
Since commit 4b050ba7a66c ("MIPS: pgtable.h: Implement the
pgprot_writecombine function for MIPS") and commit c4687b15a848 ("MIPS: Fix
definition of pgprot_writecombine()") write-combine vma mapping is
available to be used by kernel subsystems for MIPS. In particular the
uncached accelerated attribute is requested to be set by ioremap_wc()
method and by generic PCI memory pages/ranges mapping methods. The same
is done by the drm_io_prot()/ttm_io_prot() functions in case if
write-combine flag is set for vma's passed for mapping. But for some
reason the pgprot_writecombine() method calling is ifdefed to be a
platform-specific with MIPS system being marked as lacking of one. At the
very least it doesn't reflect the current MIPS platform implementation.
So in order to improve the DRM subsystem performance on MIPS with UCA
mapping enabled, we need to have pgprot_writecombine() called for buffers,
which need store operations being combined. In case if particular MIPS
chip doesn't support the UCA attribute, the mapping will fall back to
noncached.
Cc: Ralf Baechle
Cc: Paul Burton
Cc: James Hogan
Signed-off-by: Vadim V. Vlasov
Signed-off-by: Serge Semin
---
drivers/gpu/drm/drm_vm.c | 5 +++--
drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index c3301046dfaa..50178dc64060 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -62,7 +62,8 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
/* We don't want graphics memory to be mapped encrypted */
tmp = pgprot_decrypted(tmp);
-#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__)
+#if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__) || \
+defined(__mips__)
if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
tmp = pgprot_noncached(tmp);
else
@@ -73,7 +74,7 @@ static pgprot_t drm_io_prot(struct drm_local_map *map,
tmp = pgprot_writecombine(tmp);
else
tmp = pgprot_noncached(tmp);
-#elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
+#elif defined(__sparc__) || defined(__arm__)
tmp = pgprot_noncached(tmp);
#endif
return tmp;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 895d77d799e4..9f918b992f7e 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -539,13 +539,13 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp)
tmp = pgprot_noncached(tmp);
#endif
#if defined(__ia64__) || defined(__arm__) || defined(__aarch64__) || \
-defined(__powerpc__)
+defined(__powerpc__) || defined(__mips__)
if (caching_flags & TTM_PL_FLAG_WC)
tmp = pgprot_writecombine(tmp);
else
tmp = pgprot_noncached(tmp);
#endif
-#if defined(__sparc__) || defined(__mips__)
+#if defined(__sparc__)
tmp = pgprot_noncached(tmp);
#endif
return tmp;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
