Re: [linux-sunxi] Re: [PATCH] net: phy: realtek: omit setting PHY-side delay when "rgmii" specified
Icenowy, On 10/26/20 7:12 AM, Andrew Lunn wrote: >> By referring to linux/phy.h, NA means not applicable. This surely >> do not apply when RGMII is really in use. > > It means the PHY driver should not touch the mode, something else has > set it up. That could be strapping, the bootloader, ACPI firmware, > whatever. > >> I think no document declares RGMII must have all internal delays >> of the PHY explicitly disabled. It just says RGMII. > > Please take a look at all the other PHY drivers. They should all > disable delays when passed PHY_INTERFACE_MODE_RGMII. Documentation/networking/phy.rst also makes this clear: PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any internal delay by itself, it assumes that either the Ethernet MAC (if capable or the PCB traces) insert the correct 1.5-2ns delay Regards, Samuel -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/6f75be3f-90d7-982e-0f43-e742f04bb26e%40sholland.org.
[linux-sunxi] Re: [PATCH v9 01/14] ASoC: sun4i-i2s: Change set_chan_cfg() params
Hi Pierre-Louis,
On Tue, 27 Oct 2020 at 19:59, Pierre-Louis Bossart
wrote:
>
>
> > @@ -452,11 +454,11 @@ static int sun8i_i2s_set_chan_cfg(const struct
> > sun4i_i2s *i2s,
> > case SND_SOC_DAIFMT_DSP_B:
> > case SND_SOC_DAIFMT_LEFT_J:
> > case SND_SOC_DAIFMT_RIGHT_J:
> > - lrck_period = params_physical_width(params) * slots;
> > + lrck_period = slot_width * slots;
> > break;
> >
> > case SND_SOC_DAIFMT_I2S:
> > - lrck_period = params_physical_width(params);
> > + lrck_period = slot_width;
> > break;
>
> Aren't I2S, LEFT_J and RIGHT_J pretty much the same in terms of lrclk
> rate/period? the only thing that can change is the polarity, no?
>
> Not sure why it's handled differently here?
I just had a look at the User Manual for H3 and H6 and I didn't find
any reason why LEFT_J and RIGHT_J should be computed in a different
way as I2S.
Also the commit introducing this doesn't mention it.
7ae7834ec446 ("ASoC: sun4i-i2s: Add support for DSP formats")
I can't test it with my board but if nobody complains about it, I will
introduce a fix for this in the next version and change this also for
H6.
Thanks for your review,
Clement
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/CAJiuCcdX7jc-VMWYfPPL3qu9RcUU7VMdjshyPH_xLA0yVXftUw%40mail.gmail.com.
[linux-sunxi] Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation
On Tue, Oct 27, 2020 at 10:52:21AM +0100, Paul Kocialkowski wrote: > Hi, > > On Mon 26 Oct 20, 17:14, Maxime Ripard wrote: > > i2c? :) > > Oops, good catch! > > > On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote: > > > This introduces YAML bindings documentation for the A31 MIPI CSI-2 > > > controller. > > > > > > Signed-off-by: Paul Kocialkowski > > > --- > > > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 ++ > > > 1 file changed, 168 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > > > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > > new file mode 100644 > > > index ..9adc0bc27033 > > > --- /dev/null > > > +++ > > > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml > > > @@ -0,0 +1,168 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > +%YAML 1.2 > > > +--- > > > +$id: > > > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings > > > + > > > +maintainers: > > > + - Paul Kocialkowski > > > + > > > +properties: > > > + compatible: > > > +oneOf: > > > + - const: allwinner,sun6i-a31-mipi-csi2 > > > + - items: > > > + - const: allwinner,sun8i-v3s-mipi-csi2 > > > + - const: allwinner,sun6i-a31-mipi-csi2 > > > + > > > + reg: > > > +maxItems: 1 > > > + > > > + interrupts: > > > +maxItems: 1 > > > + > > > + clocks: > > > +items: > > > + - description: Bus Clock > > > + - description: Module Clock > > > + > > > + clock-names: > > > +items: > > > + - const: bus > > > + - const: mod > > > + > > > + phys: > > > +items: > > > + - description: MIPI D-PHY > > > + > > > + phy-names: > > > +items: > > > + - const: dphy > > > + > > > + resets: > > > +maxItems: 1 > > > + > > > + # See ./video-interfaces.txt for details > > > + ports: > > > +type: object > > > + > > > +properties: > > > + port@0: > > > +type: object > > > +description: Input port, connect to a MIPI CSI-2 sensor > > > + > > > +properties: > > > + reg: > > > +const: 0 > > > + > > > + endpoint: > > > +type: object > > > + > > > +properties: > > > + remote-endpoint: true > > > + > > > + bus-type: > > > +const: 4 > > > + > > > + clock-lanes: > > > +maxItems: 1 > > > + > > > + data-lanes: > > > +minItems: 1 > > > +maxItems: 4 > > > + > > > +required: > > > + - bus-type > > > + - data-lanes > > > + - remote-endpoint > > > + > > > +additionalProperties: false > > > + > > > +required: > > > + - endpoint > > > + > > > +additionalProperties: false > > > + > > > + port@1: > > > +type: object > > > +description: Output port, connect to a CSI controller > > > + > > > +properties: > > > + reg: > > > +const: 1 > > > + > > > + endpoint: > > > +type: object > > > + > > > +properties: > > > + remote-endpoint: true > > > + > > > + bus-type: > > > +const: 4 > > > > That one seems a bit weird. If the input and output ports are using the > > same format, what is that "bridge" supposed to be doing? > > Fair enough. What this represents is the internal link (likely a FIFO) between > the two controllers. It is definitely not a MIPI CSI-2 bus but there's no > mbus type for an internal link (probably because it's not a bus after all). > > Note that on the CSI controller side, we need the bus-type to be set to 4 for > it > to properly select the MIPI CSI-2 input. So it just felt more logical to have > the same on the other side of the endpoint. On the other hand, we can just > remove it on the MIPI CSI-2 controller side since it won't check it and have > it > fallback to the unknown mbus type. > > But that would make the types inconsistent on the two sides of the link. > I don't think V4L2 will complain about it at the moment, but it would also > make > sense that it does eventually. > > What do you think? There's still the same issue though, it doesn't make any sense that a bridge doesn't change the bus type. If it really did, we wouldn't need that in the first place. What you want to check in your driver is whether the subdev you're connected to has a sink pad that uses MIPI-CSI Maxime > > > +additionalProperties: false > > > + > > > +required: > > > + - endpoint > > > + > > > +
[linux-sunxi] [PATCH v9 08/14] ASoC: sun4i-i2s: fix coding-style for callback definition
Checkpatch script produces warning:
WARNING: function definition argument 'const struct sun4i_i2s *'
should also have an identifier name.
Let's fix this by adding identifier name to get_bclk_parent_rate()
and set_fmt() callback definition.
Acked-by: Maxime Ripard
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 6a3207245ae2..4cf8a67efa4f 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -180,7 +180,7 @@ struct sun4i_i2s_quirks {
const struct sun4i_i2s_clk_div *mclk_dividers;
unsigned intnum_mclk_dividers;
- unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
+ unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *i2s);
int (*get_sr)(unsigned int width);
int (*get_wss)(unsigned int width);
@@ -192,7 +192,7 @@ struct sun4i_i2s_quirks {
int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
unsigned int channels, unsigned int slots,
unsigned int slot_width);
- int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
+ int (*set_fmt)(const struct sun4i_i2s *i2s, unsigned int fmt);
};
struct sun4i_i2s {
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-9-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 03/14] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
We are actually using a complex formula to just return a bunch of
simple values. Also this formula is wrong for sun4i when calling
get_wss() the function return 4 instead of 3.
Replace this with a simpler switch case.
Also drop the i2s params which is unused and return a simple int as
returning an error code could be out of range for an s8 and there is
no optim to return a s8 here.
Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")
Reviewed-by: Chen-Yu Tsai
Acked-by: Maxime Ripard
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 69 +++--
1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9aa837d4fe99..073ee60da011 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -175,8 +175,8 @@ struct sun4i_i2s_quirks {
unsigned intnum_mclk_dividers;
unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
- s8 (*get_sr)(const struct sun4i_i2s *, int);
- s8 (*get_wss)(const struct sun4i_i2s *, int);
+ int (*get_sr)(unsigned int width);
+ int (*get_wss)(unsigned int width);
/*
* In the set_chan_cfg() function pointer:
@@ -387,37 +387,56 @@ static int sun4i_i2s_set_clk_rate(struct snd_soc_dai *dai,
return 0;
}
-static s8 sun4i_i2s_get_sr(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_sr(unsigned int width)
{
- if (width < 16 || width > 24)
- return -EINVAL;
-
- if (width % 4)
- return -EINVAL;
+ switch (width) {
+ case 16:
+ return 0;
+ case 20:
+ return 1;
+ case 24:
+ return 2;
+ }
- return (width - 16) / 4;
+ return -EINVAL;
}
-static s8 sun4i_i2s_get_wss(const struct sun4i_i2s *i2s, int width)
+static int sun4i_i2s_get_wss(unsigned int width)
{
- if (width < 16 || width > 32)
- return -EINVAL;
-
- if (width % 4)
- return -EINVAL;
+ switch (width) {
+ case 16:
+ return 0;
+ case 20:
+ return 1;
+ case 24:
+ return 2;
+ case 32:
+ return 3;
+ }
- return (width - 16) / 4;
+ return -EINVAL;
}
-static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s *i2s, int width)
+static int sun8i_i2s_get_sr_wss(unsigned int width)
{
- if (width % 4)
- return -EINVAL;
-
- if (width < 8 || width > 32)
- return -EINVAL;
+ switch (width) {
+ case 8:
+ return 1;
+ case 12:
+ return 2;
+ case 16:
+ return 3;
+ case 20:
+ return 4;
+ case 24:
+ return 5;
+ case 28:
+ return 6;
+ case 32:
+ return 7;
+ }
- return (width - 8) / 4 + 1;
+ return -EINVAL;
}
static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
@@ -582,11 +601,11 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
*substream,
}
i2s->playback_dma_data.addr_width = width;
- sr = i2s->variant->get_sr(i2s, word_size);
+ sr = i2s->variant->get_sr(word_size);
if (sr < 0)
return -EINVAL;
- wss = i2s->variant->get_wss(i2s, slot_width);
+ wss = i2s->variant->get_wss(slot_width);
if (wss < 0)
return -EINVAL;
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-4-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 07/14] ASoC: sun4i-i2s: Fix setting of FIFO modes
From: Samuel Holland
Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the
regmap is cache-only are ignored. To work around this, move the
configuration to a callback that runs while the ASoC core has a
runtime PM reference to the device.
Signed-off-by: Samuel Holland
Reviewed-by: Chen-Yu Tsai
Acked-by: Maxime Ripard
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 83537538f8ee..6a3207245ae2 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -596,6 +596,13 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
*substream,
return ret;
}
+ /* Set significant bits in our FIFOs */
+ regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
+ SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
+ SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
+
switch (params_physical_width(params)) {
case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;
@@ -924,13 +931,6 @@ static int sun4i_i2s_set_fmt(struct snd_soc_dai *dai,
unsigned int fmt)
return ret;
}
- /* Set significant bits in our FIFOs */
- regmap_update_bits(i2s->regmap, SUN4I_I2S_FIFO_CTRL_REG,
- SUN4I_I2S_FIFO_CTRL_TX_MODE_MASK |
- SUN4I_I2S_FIFO_CTRL_RX_MODE_MASK,
- SUN4I_I2S_FIFO_CTRL_TX_MODE(1) |
- SUN4I_I2S_FIFO_CTRL_RX_MODE(1));
-
i2s->format = fmt;
return 0;
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-8-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 00/14] Add Allwinner H3/H5/H6/A64 HDMI audio
Hi, This series add H6 I2S support and the I2S node missing to support HDMI audio in different Allwinner SoC. As we first use some TDM property to make the I2S working we the simple soundcard. We have now drop this simple sound card and a proper dedicated soundcard will be introduce later. This make the title of this series wrong now :/. Regards, Clement Change since v8: - move the comment near the function prototype - collect Maxime Ripard tags Change since v7: - rebase on next-20201026 - comment about slots and slot_width Change since v6: - move set_channel_cfg() in first position - convert return value to decimal Change since v5: - Drop HDMI simple soundcard - Collect Chen-Yu Tsai tags - Configure channels from 9 to 15. - Remove DMA RX for H3/H5 - Fix Documentation for H3/H5 Change since v4: - add more comment on get_wss() and set_channel_cfg() patch - merge soundcard and DAI HDMI patches Change since v3: - add Samuel Holland patch to reconfigure FIFO_TX_REG when suspend is enabled - readd inversion to H6 LRCK sun50i_h6_i2s_set_soc_fmt() - Fix get_wss() for sun4i - Add a commit to fix checkpatch warning Change since v2: - rebase on next-20200918 - drop revert LRCK polarity patch - readd simple-audio-card,frame-inversion in dts - Add patch for changing set_chan_cfg params Change since v1: - rebase on next-20200828 - add revert LRCK polarity - remove all simple-audio-card,frame-inversion in dts - add Ondrej patches for Orange Pi board - Add arm64 defconfig patch Clément Péron (6): ASoC: sun4i-i2s: Change set_chan_cfg() params ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit ASoC: sun4i-i2s: Fix sun8i volatile regs ASoC: sun4i-i2s: fix coding-style for callback definition arm64: defconfig: Enable Allwinner i2s driver dt-bindings: sound: sun4i-i2s: Document H3 with missing RX channel possibility Jernej Skrabec (3): ASoC: sun4i-i2s: Add support for H6 I2S dt-bindings: ASoC: sun4i-i2s: Add H6 compatible arm64: dts: allwinner: h6: Add I2S1 node Marcus Cooper (4): ASoC: sun4i-i2s: Set sign extend sample ASoc: sun4i-i2s: Add 20 and 24 bit support arm64: dts: allwinner: a64: Add I2S2 node arm: dts: sunxi: h3/h5: Add I2S2 node Samuel Holland (1): ASoC: sun4i-i2s: Fix setting of FIFO modes .../sound/allwinner,sun4i-a10-i2s.yaml| 6 +- arch/arm/boot/dts/sunxi-h3-h5.dtsi| 13 + arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 + arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 13 + arch/arm64/configs/defconfig | 1 + sound/soc/sunxi/sun4i-i2s.c | 384 +++--- 6 files changed, 376 insertions(+), 55 deletions(-) -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-1-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 14/14] arm: dts: sunxi: h3/h5: Add I2S2 node
From: Marcus Cooper
Add H3/H5 I2S2 node connected to the HDMI interface.
Signed-off-by: Jernej Skrabec
Signed-off-by: Marcus Cooper
Acked-by: Chen-Yu Tsai
Signed-off-by: Clément Péron
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 22d533d18992..9be13378d4df 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -662,6 +662,19 @@ i2s1: i2s@1c22400 {
status = "disabled";
};
+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun8i-h3-i2s";
+ reg = <0x01c22800 0x400>;
+ interrupts = ;
+ clocks = < CLK_BUS_I2S2>, < CLK_I2S2>;
+ clock-names = "apb", "mod";
+ dmas = < 27>;
+ resets = < RST_BUS_I2S2>;
+ dma-names = "tx";
+ status = "disabled";
+ };
+
codec: codec@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun8i-h3-codec";
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-15-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 01/14] ASoC: sun4i-i2s: Change set_chan_cfg() params
As slots and slot_width can be set manually using set_tdm().
These values are then kept in sun4i_i2s struct.
So we need to check if these values are set or not.
This is not done actually and will trigger a bug.
For example, if we set to the simple soundcard in the device-tree
dai-tdm-slot-width = <32> and then start a stream using S16_LE,
currently we would calculate BCLK for 32-bit slots, but program
lrck_period for 16-bit slots, making the sample rate double what we
expected.
To fix this, we need to check if these values are set or not but as
this logic is already done by the caller. Avoid duplicating this
logic and just pass the required values as params to set_chan_cfg().
Suggested-by: Samuel Holland
Acked-by: Maxime Ripard
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index f23ff29e7c1d..7c1f57eb2462 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -162,8 +162,15 @@ struct sun4i_i2s_quirks {
unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
s8 (*get_sr)(const struct sun4i_i2s *, int);
s8 (*get_wss)(const struct sun4i_i2s *, int);
- int (*set_chan_cfg)(const struct sun4i_i2s *,
- const struct snd_pcm_hw_params *);
+
+ /*
+* In the set_chan_cfg() function pointer:
+* @slots: channels per frame + padding slots, regardless of format
+* @slot_width: bits per sample + padding bits, regardless of format
+*/
+ int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width);
int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
};
@@ -399,10 +406,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s
*i2s, int width)
}
static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x3210);
@@ -419,15 +425,11 @@ static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s
*i2s,
}
static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
- const struct snd_pcm_hw_params *params)
+ unsigned int channels, unsigned int slots,
+ unsigned int slot_width)
{
- unsigned int channels = params_channels(params);
- unsigned int slots = channels;
unsigned int lrck_period;
- if (i2s->slots)
- slots = i2s->slots;
-
/* Map the channels for playback and capture */
regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
@@ -452,11 +454,11 @@ static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s
*i2s,
case SND_SOC_DAIFMT_DSP_B:
case SND_SOC_DAIFMT_LEFT_J:
case SND_SOC_DAIFMT_RIGHT_J:
- lrck_period = params_physical_width(params) * slots;
+ lrck_period = slot_width * slots;
break;
case SND_SOC_DAIFMT_I2S:
- lrck_period = params_physical_width(params);
+ lrck_period = slot_width;
break;
default:
@@ -482,7 +484,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
*substream,
unsigned int word_size = params_width(params);
unsigned int slot_width = params_physical_width(params);
unsigned int channels = params_channels(params);
+
unsigned int slots = channels;
+
int ret, sr, wss;
u32 width;
@@ -492,7 +496,7 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
*substream,
if (i2s->slot_width)
slot_width = i2s->slot_width;
- ret = i2s->variant->set_chan_cfg(i2s, params);
+ ret = i2s->variant->set_chan_cfg(i2s, channels, slots, slot_width);
if (ret < 0) {
dev_err(dai->dev, "Invalid channel configuration\n");
return ret;
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
[linux-sunxi] [PATCH v9 06/14] ASoC: sun4i-i2s: Fix sun8i volatile regs
The FIFO TX reg is volatile and sun8i i2s register
mapping is different from sun4i.
Even if in this case it's doesn't create an issue,
Avoid setting some regs that are undefined in sun8i.
Acked-by: Maxime Ripard
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 39b56d0de1fd..83537538f8ee 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1162,12 +1162,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev,
unsigned int reg)
static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg)
{
- if (reg == SUN8I_I2S_INT_STA_REG)
+ switch (reg) {
+ case SUN4I_I2S_FIFO_CTRL_REG:
+ case SUN4I_I2S_FIFO_RX_REG:
+ case SUN4I_I2S_FIFO_STA_REG:
+ case SUN4I_I2S_RX_CNT_REG:
+ case SUN4I_I2S_TX_CNT_REG:
+ case SUN8I_I2S_FIFO_TX_REG:
+ case SUN8I_I2S_INT_STA_REG:
return true;
- if (reg == SUN8I_I2S_FIFO_TX_REG)
- return false;
- return sun4i_i2s_volatile_reg(dev, reg);
+ default:
+ return false;
+ }
}
static const struct reg_default sun4i_i2s_reg_defaults[] = {
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-7-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 11/14] arm64: dts: allwinner: a64: Add I2S2 node
From: Marcus Cooper
Add the I2S2 node connected to the HDMI interface.
Signed-off-by: Jernej Skrabec
Signed-off-by: Marcus Cooper
Acked-by: Chen-Yu Tsai
Signed-off-by: Clément Péron
---
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index dc238814013c..51cc30e84e26 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -846,6 +846,20 @@ i2s1: i2s@1c22400 {
status = "disabled";
};
+ i2s2: i2s@1c22800 {
+ #sound-dai-cells = <0>;
+ compatible = "allwinner,sun50i-a64-i2s",
+"allwinner,sun8i-h3-i2s";
+ reg = <0x01c22800 0x400>;
+ interrupts = ;
+ clocks = < CLK_BUS_I2S2>, < CLK_I2S2>;
+ clock-names = "apb", "mod";
+ resets = < RST_BUS_I2S2>;
+ dma-names = "rx", "tx";
+ dmas = < 27>, < 27>;
+ status = "disabled";
+ };
+
dai: dai@1c22c00 {
#sound-dai-cells = <0>;
compatible = "allwinner,sun50i-a64-codec-i2s";
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-12-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 13/14] dt-bindings: sound: sun4i-i2s: Document H3 with missing RX channel possibility
Like A83T the Allwinner H3 doesn't have the DMA reception available for some audio interfaces. As it's already documented for A83T convert this to an enum and add the H3 interface. Acked-by: Rob Herring Signed-off-by: Clément Péron --- .../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml| 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml index 606ad2d884a8..a16e37b01e1d 100644 --- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml @@ -70,7 +70,9 @@ allOf: properties: compatible: contains: -const: allwinner,sun8i-a83t-i2s +enum: + - allwinner,sun8i-a83t-i2s + - allwinner,sun8i-h3-i2s then: properties: -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-14-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 09/14] dt-bindings: ASoC: sun4i-i2s: Add H6 compatible
From: Jernej Skrabec H6 I2S is very similar to H3, except that it supports up to 16 channels and thus few registers have fields on different position. Signed-off-by: Jernej Skrabec Signed-off-by: Marcus Cooper Acked-by: Maxime Ripard Acked-by: Rob Herring Acked-by: Chen-Yu Tsai Signed-off-by: Clément Péron --- .../devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml index 112ae00d63c1..606ad2d884a8 100644 --- a/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/allwinner,sun4i-a10-i2s.yaml @@ -24,6 +24,7 @@ properties: - items: - const: allwinner,sun50i-a64-i2s - const: allwinner,sun8i-h3-i2s + - const: allwinner,sun50i-h6-i2s reg: maxItems: 1 @@ -59,6 +60,7 @@ allOf: - allwinner,sun8i-a83t-i2s - allwinner,sun8i-h3-i2s - allwinner,sun50i-a64-codec-i2s + - allwinner,sun50i-h6-i2s then: required: -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-10-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 05/14] ASoc: sun4i-i2s: Add 20 and 24 bit support
From: Marcus Cooper
Extend the functionality of the driver to include support of 20 and
24 bits per sample.
Signed-off-by: Marcus Cooper
Acked-by: Maxime Ripard
Reviewed-by: Chen-Yu Tsai
Signed-off-by: Clément Péron
---
sound/soc/sunxi/sun4i-i2s.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 9f9d3e7baad0..39b56d0de1fd 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -600,6 +600,9 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
*substream,
case 16:
width = DMA_SLAVE_BUSWIDTH_2_BYTES;
break;
+ case 32:
+ width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+ break;
default:
dev_err(dai->dev, "Unsupported physical sample width: %d\n",
params_physical_width(params));
@@ -1081,6 +1084,10 @@ static int sun4i_i2s_dai_probe(struct snd_soc_dai *dai)
return 0;
}
+#define SUN4I_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+SNDRV_PCM_FMTBIT_S20_LE | \
+SNDRV_PCM_FMTBIT_S24_LE)
+
static struct snd_soc_dai_driver sun4i_i2s_dai = {
.probe = sun4i_i2s_dai_probe,
.capture = {
@@ -1088,14 +1095,14 @@ static struct snd_soc_dai_driver sun4i_i2s_dai = {
.channels_min = 1,
.channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_192000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SUN4I_FORMATS,
},
.playback = {
.stream_name = "Playback",
.channels_min = 1,
.channels_max = 8,
.rates = SNDRV_PCM_RATE_8000_192000,
- .formats = SNDRV_PCM_FMTBIT_S16_LE,
+ .formats = SUN4I_FORMATS,
},
.ops = _i2s_dai_ops,
.symmetric_rates = 1,
--
2.25.1
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-6-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 04/14] ASoC: sun4i-i2s: Set sign extend sample
From: Marcus Cooper On the newer SoCs such as the H3 and A64 this is set by default to transfer a 0 after each sample in each slot. However the A10 and A20 SoCs that this driver was developed on had a default setting where it padded the audio gain with zeros. This isn't a problem while we have only support for 16bit audio but with larger sample resolution rates in the pipeline then SEXT bits should be cleared so that they also pad at the LSB. Without this the audio gets distorted. Set sign extend sample for all the sunxi generations even if they are not affected. This will keep consistency and avoid relying on default. Signed-off-by: Marcus Cooper Reviewed-by: Chen-Yu Tsai Acked-by: Maxime Ripard Signed-off-by: Clément Péron --- sound/soc/sunxi/sun4i-i2s.c | 17 + 1 file changed, 17 insertions(+) diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 073ee60da011..9f9d3e7baad0 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -48,6 +48,9 @@ #define SUN4I_I2S_FMT0_FMT_I2S (0 << 0) #define SUN4I_I2S_FMT1_REG 0x08 +#define SUN4I_I2S_FMT1_REG_SEXT_MASK BIT(8) +#define SUN4I_I2S_FMT1_REG_SEXT(sext) ((sext) << 8) + #define SUN4I_I2S_FIFO_TX_REG 0x0c #define SUN4I_I2S_FIFO_RX_REG 0x10 @@ -105,6 +108,9 @@ #define SUN8I_I2S_FMT0_BCLK_POLARITY_INVERTED (1 << 7) #define SUN8I_I2S_FMT0_BCLK_POLARITY_NORMAL(0 << 7) +#define SUN8I_I2S_FMT1_REG_SEXT_MASK GENMASK(5, 4) +#define SUN8I_I2S_FMT1_REG_SEXT(sext) ((sext) << 4) + #define SUN8I_I2S_INT_STA_REG 0x0c #define SUN8I_I2S_FIFO_TX_REG 0x20 @@ -686,6 +692,7 @@ static int sun4i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s, } regmap_update_bits(i2s->regmap, SUN4I_I2S_CTRL_REG, SUN4I_I2S_CTRL_MODE_MASK, val); + return 0; } @@ -788,6 +795,11 @@ static int sun8i_i2s_set_soc_fmt(const struct sun4i_i2s *i2s, SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT, val); + /* Set sign extension to pad out LSB with 0 */ + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG, + SUN8I_I2S_FMT1_REG_SEXT_MASK, + SUN8I_I2S_FMT1_REG_SEXT(0)); + return 0; } @@ -890,6 +902,11 @@ static int sun50i_h6_i2s_set_soc_fmt(const struct sun4i_i2s *i2s, SUN8I_I2S_CTRL_BCLK_OUT | SUN8I_I2S_CTRL_LRCK_OUT, val); + /* Set sign extension to pad out LSB with 0 */ + regmap_update_bits(i2s->regmap, SUN4I_I2S_FMT1_REG, + SUN8I_I2S_FMT1_REG_SEXT_MASK, + SUN8I_I2S_FMT1_REG_SEXT(0)); + return 0; } -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-5-peron.clem%40gmail.com.
[linux-sunxi] [PATCH v9 12/14] arm64: defconfig: Enable Allwinner i2s driver
Enable Allwinner I2S driver for arm64 defconfig. Signed-off-by: Clément Péron --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 17a2df6a263e..3f89f427a355 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -706,6 +706,7 @@ CONFIG_SND_SOC_ROCKCHIP_RT5645=m CONFIG_SND_SOC_RK3399_GRU_SOUND=m CONFIG_SND_SOC_SAMSUNG=y CONFIG_SND_SOC_RCAR=m +CONFIG_SND_SUN4I_I2S=m CONFIG_SND_SUN4I_SPDIF=m CONFIG_SND_SOC_TEGRA=m CONFIG_SND_SOC_TEGRA210_AHUB=m -- 2.25.1 -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027183149.145165-13-peron.clem%40gmail.com.
[linux-sunxi] Re: [PATCH 05/14] media: sun6i-csi: Only configure the interface data width for parallel
On Tue, Oct 27, 2020 at 10:31:19AM +0100, Paul Kocialkowski wrote:
> Hi,
>
> On Mon 26 Oct 20, 17:00, Maxime Ripard wrote:
> > On Fri, Oct 23, 2020 at 07:45:37PM +0200, Paul Kocialkowski wrote:
> > > Bits related to the interface data width do not have any effect when
> > > the CSI controller is taking input from the MIPI CSI-2 controller.
> >
> > I guess it would be clearer to mention that the data width is only
> > applicable for parallel here.
>
> Understood, will change the wording in the next version.
>
> > > In prevision of adding support for this case, set these bits
> > > conditionally so there is no ambiguity.
> > >
> > > Co-developed-by: Kévin L'hôpital
> > > Signed-off-by: Kévin L'hôpital
> > > Signed-off-by: Paul Kocialkowski
> > > ---
> > > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 42 +++
> > > 1 file changed, 25 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > index 5d2389a5cd17..a876a05ea3c7 100644
> > > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > > @@ -378,8 +378,13 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > > *sdev)
> > > unsigned char bus_width;
> > > u32 flags;
> > > u32 cfg;
> > > + bool input_parallel = false;
> > > bool input_interlaced = false;
> > >
> > > + if (endpoint->bus_type == V4L2_MBUS_PARALLEL ||
> > > + endpoint->bus_type == V4L2_MBUS_BT656)
> > > + input_parallel = true;
> > > +
> > > if (csi->config.field == V4L2_FIELD_INTERLACED
> > > || csi->config.field == V4L2_FIELD_INTERLACED_TB
> > > || csi->config.field == V4L2_FIELD_INTERLACED_BT)
> > > @@ -395,6 +400,26 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > > *sdev)
> > >CSI_IF_CFG_HREF_POL_MASK | CSI_IF_CFG_FIELD_MASK |
> > >CSI_IF_CFG_SRC_TYPE_MASK);
> > >
> > > + if (input_parallel) {
> > > + switch (bus_width) {
> > > + case 8:
> > > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_8BIT;
> > > + break;
> > > + case 10:
> > > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_10BIT;
> > > + break;
> > > + case 12:
> > > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_12BIT;
> > > + break;
> > > + case 16: /* No need to configure DATA_WIDTH for 16bit */
> > > + break;
> > > + default:
> > > + dev_warn(sdev->dev, "Unsupported bus width: %u\n",
> > > + bus_width);
> > > + break;
> > > + }
> > > + }
> > > +
> > > if (input_interlaced)
> > > cfg |= CSI_IF_CFG_SRC_TYPE_INTERLACED;
> > > else
> > > @@ -440,23 +465,6 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > > *sdev)
> > > break;
> > > }
> > >
> > > - switch (bus_width) {
> > > - case 8:
> > > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_8BIT;
> > > - break;
> > > - case 10:
> > > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_10BIT;
> > > - break;
> > > - case 12:
> > > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_12BIT;
> > > - break;
> > > - case 16: /* No need to configure DATA_WIDTH for 16bit */
> > > - break;
> > > - default:
> > > - dev_warn(sdev->dev, "Unsupported bus width: %u\n", bus_width);
> > > - break;
> > > - }
> > > -
> >
> > Is there any reason to move it around?
>
> The main reason is cosmetics: input_parallel is introduced to match the
> already
> existing input_interlaced variable, so it made sense to me to have both of
> these
> conditionals one after the other instead of spreading them randomly.
>
> I can mention this in the commit log if you prefer.
Yeah, that would great
Maxime
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027183107.sofqfbmgg5aancmr%40gilmour.lan.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH 02/14] phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI CSI-2
Hi,
On Tue, Oct 27, 2020 at 10:23:26AM +0100, Paul Kocialkowski wrote:
> On Mon 26 Oct 20, 16:38, Maxime Ripard wrote:
> > On Fri, Oct 23, 2020 at 07:45:34PM +0200, Paul Kocialkowski wrote:
> > > The Allwinner A31 D-PHY supports both Rx and Tx modes. While the latter
> > > is already supported and used for MIPI DSI this adds support for the
> > > former, to be used with MIPI CSI-2.
> > >
> > > This implementation is inspired by the Allwinner BSP implementation.
> >
> > Mentionning which BSP you took this from would be helpful
>
> Sure! It's from the Github repo linked from https://linux-sunxi.org/V3s.
> Would you like that I mention this URL explicitly or would it be enough to
> mention "Allwinner's V3s Linux SDK" as they seem to call it?
Yeah, that would be great
> > > +static int sun6i_dphy_rx_power_on(struct sun6i_dphy *dphy)
> > > +{
> > > + /* Physical clock rate is actually half of symbol rate with DDR. */
> > > + unsigned long mipi_symbol_rate = dphy->config.hs_clk_rate;
> > > + unsigned long dphy_clk_rate;
> > > + unsigned int rx_dly;
> > > + unsigned int lprst_dly;
> > > + u32 value;
> > > +
> > > + dphy_clk_rate = clk_get_rate(dphy->mod_clk);
> > > + if (!dphy_clk_rate)
> > > + return -1;
> >
> > Returning -1 is weird here?
>
> What do you think would be a more appropriate error code to return?
> It looks like some other drivers return -EINVAL when that happens (but many
> don't do the check).
Yeah, EINVAL at least is better than ENOPERM
> > > +
> > > + /* Hardcoded timing parameters from the Allwinner BSP. */
> > > + regmap_write(dphy->regs, SUN6I_DPHY_RX_TIME0_REG,
> > > + SUN6I_DPHY_RX_TIME0_HS_RX_SYNC(255) |
> > > + SUN6I_DPHY_RX_TIME0_HS_RX_CLK_MISS(255) |
> > > + SUN6I_DPHY_RX_TIME0_LP_RX(255));
> > > +
> > > + /*
> > > + * Formula from the Allwinner BSP, with hardcoded coefficients
> > > + * (probably internal divider/multiplier).
> > > + */
> > > + rx_dly = 8 * (unsigned int)(dphy_clk_rate / (mipi_symbol_rate / 8));
> > > +
> > > + /*
> > > + * The Allwinner BSP has an alternative formula for LP_RX_ULPS_WP:
> > > + * lp_ulps_wp_cnt = lp_ulps_wp_ms * lp_clk / 1000
> > > + * but does not use it and hardcodes 255 instead.
> > > + */
> > > + regmap_write(dphy->regs, SUN6I_DPHY_RX_TIME1_REG,
> > > + SUN6I_DPHY_RX_TIME1_RX_DLY(rx_dly) |
> > > + SUN6I_DPHY_RX_TIME1_LP_RX_ULPS_WP(255));
> > > +
> > > + /* HS_RX_ANA0 value is hardcoded in the Allwinner BSP. */
> > > + regmap_write(dphy->regs, SUN6I_DPHY_RX_TIME2_REG,
> > > + SUN6I_DPHY_RX_TIME2_HS_RX_ANA0(4));
> > > +
> > > + /*
> > > + * Formula from the Allwinner BSP, with hardcoded coefficients
> > > + * (probably internal divider/multiplier).
> > > + */
> > > + lprst_dly = 4 * (unsigned int)(dphy_clk_rate / (mipi_symbol_rate / 2));
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_RX_TIME3_REG,
> > > + SUN6I_DPHY_RX_TIME3_LPRST_DLY(lprst_dly));
> > > +
> > > + /* Analog parameters are hardcoded in the Allwinner BSP. */
> > > + regmap_write(dphy->regs, SUN6I_DPHY_ANA0_REG,
> > > + SUN6I_DPHY_ANA0_REG_PWS |
> > > + SUN6I_DPHY_ANA0_REG_SLV(7) |
> > > + SUN6I_DPHY_ANA0_REG_SFB(2));
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_ANA1_REG,
> > > + SUN6I_DPHY_ANA1_REG_SVTT(4));
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_ANA4_REG,
> > > + SUN6I_DPHY_ANA4_REG_DMPLVC |
> > > + SUN6I_DPHY_ANA4_REG_DMPLVD(1));
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_ANA2_REG,
> > > + SUN6I_DPHY_ANA2_REG_ENIB);
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_ANA3_REG,
> > > + SUN6I_DPHY_ANA3_EN_LDOR |
> > > + SUN6I_DPHY_ANA3_EN_LDOC |
> > > + SUN6I_DPHY_ANA3_EN_LDOD);
> > > +
> > > + /*
> > > + * Delay comes from the Allwinner BSP, likely for internal regulator
> > > + * ramp-up.
> > > + */
> > > + udelay(3);
> > > +
> > > + value = SUN6I_DPHY_RX_CTL_EN_DBC | SUN6I_DPHY_RX_CTL_RX_CLK_FORCE;
> > > +
> > > + /*
> > > + * Rx data lane force-enable bits are used as regular RX enable by the
> > > + * Allwinner BSP.
> > > + */
> > > + if (dphy->config.lanes >= 1)
> > > + value |= SUN6I_DPHY_RX_CTL_RX_D0_FORCE;
> > > + if (dphy->config.lanes >= 2)
> > > + value |= SUN6I_DPHY_RX_CTL_RX_D1_FORCE;
> > > + if (dphy->config.lanes >= 3)
> > > + value |= SUN6I_DPHY_RX_CTL_RX_D2_FORCE;
> > > + if (dphy->config.lanes == 4)
> > > + value |= SUN6I_DPHY_RX_CTL_RX_D3_FORCE;
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_RX_CTL_REG, value);
> > > +
> > > + regmap_write(dphy->regs, SUN6I_DPHY_GCTL_REG,
> > > + SUN6I_DPHY_GCTL_LANE_NUM(dphy->config.lanes) |
> > > + SUN6I_DPHY_GCTL_EN);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int sun6i_dphy_power_on(struct phy *phy)
> > > +{
> > > + struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> >
[linux-sunxi] Re: [PATCH v8 07/14] ASoC: sun4i-i2s: Fix setting of FIFO modes
On Mon, Oct 26, 2020 at 07:52:32PM +0100, Clément Péron wrote: > From: Samuel Holland > > Because SUN4I_I2S_FIFO_CTRL_REG is volatile, writes done while the > regmap is cache-only are ignored. To work around this, move the > configuration to a callback that runs while the ASoC core has a > runtime PM reference to the device. > > Signed-off-by: Samuel Holland > Reviewed-by: Chen-Yu Tsai > Signed-off-by: Clément Péron Acked-by: Maxime Ripard Maxime -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027175133.s23vdqxnnt3cluip%40gilmour.lan. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v8 04/14] ASoC: sun4i-i2s: Set sign extend sample
On Mon, Oct 26, 2020 at 07:52:29PM +0100, Clément Péron wrote: > From: Marcus Cooper > > On the newer SoCs such as the H3 and A64 this is set by default > to transfer a 0 after each sample in each slot. However the A10 > and A20 SoCs that this driver was developed on had a default > setting where it padded the audio gain with zeros. > > This isn't a problem while we have only support for 16bit audio > but with larger sample resolution rates in the pipeline then SEXT > bits should be cleared so that they also pad at the LSB. Without > this the audio gets distorted. > > Set sign extend sample for all the sunxi generations even if they > are not affected. This will keep consistency and avoid relying on > default. > > Signed-off-by: Marcus Cooper > Reviewed-by: Chen-Yu Tsai > Signed-off-by: Clément Péron Acked-by: Maxime Ripard Maxime -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027175032.utw23rtbds6fwrbw%40gilmour.lan. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v8 03/14] ASoC: sun4i-i2s: Change get_sr() and get_wss() to be more explicit
On Mon, Oct 26, 2020 at 07:52:28PM +0100, Clément Péron wrote:
> We are actually using a complex formula to just return a bunch of
> simple values. Also this formula is wrong for sun4i when calling
> get_wss() the function return 4 instead of 3.
>
> Replace this with a simpler switch case.
>
> Also drop the i2s params which is unused and return a simple int as
> returning an error code could be out of range for an s8 and there is
> no optim to return a s8 here.
>
> Fixes: 619c15f7fac9 ("ASoC: sun4i-i2s: Change SR and WSS computation")
> Reviewed-by: Chen-Yu Tsai
> Signed-off-by: Clément Péron
Acked-by: Maxime Ripard
Maxime
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027175016.ffx7lokjbscczvox%40gilmour.lan.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH v8 02/14] ASoC: sun4i-i2s: Add support for H6 I2S
On Mon, Oct 26, 2020 at 07:52:27PM +0100, Clément Péron wrote: > From: Jernej Skrabec > > H6 I2S is very similar to that in H3, except it supports up to 16 > channels. > > Signed-off-by: Jernej Skrabec > Signed-off-by: Marcus Cooper > Reviewed-by: Chen-Yu Tsai > Signed-off-by: Clément Péron Acked-by: Maxime Ripard Maxime -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20201027174957.ui4ootubgb46dqgv%40gilmour.lan. signature.asc Description: PGP signature
[linux-sunxi] Re: [PATCH v8 01/14] ASoC: sun4i-i2s: Change set_chan_cfg() params
On Mon, Oct 26, 2020 at 07:52:26PM +0100, Clément Péron wrote:
> As slots and slot_width can be set manually using set_tdm().
> These values are then kept in sun4i_i2s struct.
> So we need to check if these values are set or not.
>
> This is not done actually and will trigger a bug.
> For example, if we set to the simple soundcard in the device-tree
> dai-tdm-slot-width = <32> and then start a stream using S16_LE,
> currently we would calculate BCLK for 32-bit slots, but program
> lrck_period for 16-bit slots, making the sample rate double what we
> expected.
>
> To fix this, we need to check if these values are set or not but as
> this logic is already done by the caller. Avoid duplicating this
> logic and just pass the required values as params to set_chan_cfg().
>
> Suggested-by: Samuel Holland
> Signed-off-by: Clément Péron
> ---
> sound/soc/sunxi/sun4i-i2s.c | 33 ++---
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index f23ff29e7c1d..6c10f810b114 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -162,8 +162,9 @@ struct sun4i_i2s_quirks {
> unsigned long (*get_bclk_parent_rate)(const struct sun4i_i2s *);
> s8 (*get_sr)(const struct sun4i_i2s *, int);
> s8 (*get_wss)(const struct sun4i_i2s *, int);
> - int (*set_chan_cfg)(const struct sun4i_i2s *,
> - const struct snd_pcm_hw_params *);
> + int (*set_chan_cfg)(const struct sun4i_i2s *i2s,
> + unsigned int channels, unsigned int slots,
> + unsigned int slot_width);
> int (*set_fmt)(const struct sun4i_i2s *, unsigned int);
> };
>
> @@ -399,10 +400,9 @@ static s8 sun8i_i2s_get_sr_wss(const struct sun4i_i2s
> *i2s, int width)
> }
>
> static int sun4i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> - const struct snd_pcm_hw_params *params)
> + unsigned int channels, unsigned int slots,
> + unsigned int slot_width)
> {
> - unsigned int channels = params_channels(params);
> -
> /* Map the channels for playback and capture */
> regmap_write(i2s->regmap, SUN4I_I2S_TX_CHAN_MAP_REG, 0x76543210);
> regmap_write(i2s->regmap, SUN4I_I2S_RX_CHAN_MAP_REG, 0x3210);
> @@ -419,15 +419,11 @@ static int sun4i_i2s_set_chan_cfg(const struct
> sun4i_i2s *i2s,
> }
>
> static int sun8i_i2s_set_chan_cfg(const struct sun4i_i2s *i2s,
> - const struct snd_pcm_hw_params *params)
> + unsigned int channels, unsigned int slots,
> + unsigned int slot_width)
> {
> - unsigned int channels = params_channels(params);
> - unsigned int slots = channels;
> unsigned int lrck_period;
>
> - if (i2s->slots)
> - slots = i2s->slots;
> -
> /* Map the channels for playback and capture */
> regmap_write(i2s->regmap, SUN8I_I2S_TX_CHAN_MAP_REG, 0x76543210);
> regmap_write(i2s->regmap, SUN8I_I2S_RX_CHAN_MAP_REG, 0x76543210);
> @@ -452,11 +448,11 @@ static int sun8i_i2s_set_chan_cfg(const struct
> sun4i_i2s *i2s,
> case SND_SOC_DAIFMT_DSP_B:
> case SND_SOC_DAIFMT_LEFT_J:
> case SND_SOC_DAIFMT_RIGHT_J:
> - lrck_period = params_physical_width(params) * slots;
> + lrck_period = slot_width * slots;
> break;
>
> case SND_SOC_DAIFMT_I2S:
> - lrck_period = params_physical_width(params);
> + lrck_period = slot_width;
> break;
>
> default:
> @@ -480,9 +476,16 @@ static int sun4i_i2s_hw_params(struct snd_pcm_substream
> *substream,
> {
> struct sun4i_i2s *i2s = snd_soc_dai_get_drvdata(dai);
> unsigned int word_size = params_width(params);
> - unsigned int slot_width = params_physical_width(params);
> unsigned int channels = params_channels(params);
> +
> + /*
> + * Here and in set_chan_cfg(), "slots" means channels per frame +
> + * padding slots, regardless of format. "slot_width" means bits
> + * per sample + padding bits, regardless of format.
> + */
> unsigned int slots = channels;
> + unsigned int slot_width = params_physical_width(params);
> +
what I meant was to put that comment next to the function pointer in the
structure sun4i_i2s_quirks, it would be fairly easy to miss here.
With that fixed,
Acked-by: Maxime Ripard
Maxime
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027174917.xushgbmiohxwydnh%40gilmour.lan.
[linux-sunxi] Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2 bindings documentation
Hi,
On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> i2c? :)
Oops, good catch!
> On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > controller.
> >
> > Signed-off-by: Paul Kocialkowski
> > ---
> > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 ++
> > 1 file changed, 168 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> >
> > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > new file mode 100644
> > index ..9adc0bc27033
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > @@ -0,0 +1,168 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > +
> > +maintainers:
> > + - Paul Kocialkowski
> > +
> > +properties:
> > + compatible:
> > +oneOf:
> > + - const: allwinner,sun6i-a31-mipi-csi2
> > + - items:
> > + - const: allwinner,sun8i-v3s-mipi-csi2
> > + - const: allwinner,sun6i-a31-mipi-csi2
> > +
> > + reg:
> > +maxItems: 1
> > +
> > + interrupts:
> > +maxItems: 1
> > +
> > + clocks:
> > +items:
> > + - description: Bus Clock
> > + - description: Module Clock
> > +
> > + clock-names:
> > +items:
> > + - const: bus
> > + - const: mod
> > +
> > + phys:
> > +items:
> > + - description: MIPI D-PHY
> > +
> > + phy-names:
> > +items:
> > + - const: dphy
> > +
> > + resets:
> > +maxItems: 1
> > +
> > + # See ./video-interfaces.txt for details
> > + ports:
> > +type: object
> > +
> > +properties:
> > + port@0:
> > +type: object
> > +description: Input port, connect to a MIPI CSI-2 sensor
> > +
> > +properties:
> > + reg:
> > +const: 0
> > +
> > + endpoint:
> > +type: object
> > +
> > +properties:
> > + remote-endpoint: true
> > +
> > + bus-type:
> > +const: 4
> > +
> > + clock-lanes:
> > +maxItems: 1
> > +
> > + data-lanes:
> > +minItems: 1
> > +maxItems: 4
> > +
> > +required:
> > + - bus-type
> > + - data-lanes
> > + - remote-endpoint
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - endpoint
> > +
> > +additionalProperties: false
> > +
> > + port@1:
> > +type: object
> > +description: Output port, connect to a CSI controller
> > +
> > +properties:
> > + reg:
> > +const: 1
> > +
> > + endpoint:
> > +type: object
> > +
> > +properties:
> > + remote-endpoint: true
> > +
> > + bus-type:
> > +const: 4
>
> That one seems a bit weird. If the input and output ports are using the
> same format, what is that "bridge" supposed to be doing?
Fair enough. What this represents is the internal link (likely a FIFO) between
the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
mbus type for an internal link (probably because it's not a bus after all).
Note that on the CSI controller side, we need the bus-type to be set to 4 for it
to properly select the MIPI CSI-2 input. So it just felt more logical to have
the same on the other side of the endpoint. On the other hand, we can just
remove it on the MIPI CSI-2 controller side since it won't check it and have it
fallback to the unknown mbus type.
But that would make the types inconsistent on the two sides of the link.
I don't think V4L2 will complain about it at the moment, but it would also make
sense that it does eventually.
What do you think?
> > +additionalProperties: false
> > +
> > +required:
> > + - endpoint
> > +
> > +additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +#include
> > +#include
> > +
> > +mipi_csi2: mipi-csi2@1cb1000 {
>
> The unit name should be pretty standard, with the list here:
>
> https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst#generic-names-recommendation
>
> there's nothing really standing out for us in that list, but given that
> there's dsi, we should stick with csi
Then what really
[linux-sunxi] Re: [PATCH 05/14] media: sun6i-csi: Only configure the interface data width for parallel
Hi,
On Mon 26 Oct 20, 17:00, Maxime Ripard wrote:
> On Fri, Oct 23, 2020 at 07:45:37PM +0200, Paul Kocialkowski wrote:
> > Bits related to the interface data width do not have any effect when
> > the CSI controller is taking input from the MIPI CSI-2 controller.
>
> I guess it would be clearer to mention that the data width is only
> applicable for parallel here.
Understood, will change the wording in the next version.
> > In prevision of adding support for this case, set these bits
> > conditionally so there is no ambiguity.
> >
> > Co-developed-by: Kévin L'hôpital
> > Signed-off-by: Kévin L'hôpital
> > Signed-off-by: Paul Kocialkowski
> > ---
> > .../platform/sunxi/sun6i-csi/sun6i_csi.c | 42 +++
> > 1 file changed, 25 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > index 5d2389a5cd17..a876a05ea3c7 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > @@ -378,8 +378,13 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > *sdev)
> > unsigned char bus_width;
> > u32 flags;
> > u32 cfg;
> > + bool input_parallel = false;
> > bool input_interlaced = false;
> >
> > + if (endpoint->bus_type == V4L2_MBUS_PARALLEL ||
> > + endpoint->bus_type == V4L2_MBUS_BT656)
> > + input_parallel = true;
> > +
> > if (csi->config.field == V4L2_FIELD_INTERLACED
> > || csi->config.field == V4L2_FIELD_INTERLACED_TB
> > || csi->config.field == V4L2_FIELD_INTERLACED_BT)
> > @@ -395,6 +400,26 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > *sdev)
> > CSI_IF_CFG_HREF_POL_MASK | CSI_IF_CFG_FIELD_MASK |
> > CSI_IF_CFG_SRC_TYPE_MASK);
> >
> > + if (input_parallel) {
> > + switch (bus_width) {
> > + case 8:
> > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_8BIT;
> > + break;
> > + case 10:
> > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_10BIT;
> > + break;
> > + case 12:
> > + cfg |= CSI_IF_CFG_IF_DATA_WIDTH_12BIT;
> > + break;
> > + case 16: /* No need to configure DATA_WIDTH for 16bit */
> > + break;
> > + default:
> > + dev_warn(sdev->dev, "Unsupported bus width: %u\n",
> > +bus_width);
> > + break;
> > + }
> > + }
> > +
> > if (input_interlaced)
> > cfg |= CSI_IF_CFG_SRC_TYPE_INTERLACED;
> > else
> > @@ -440,23 +465,6 @@ static void sun6i_csi_setup_bus(struct sun6i_csi_dev
> > *sdev)
> > break;
> > }
> >
> > - switch (bus_width) {
> > - case 8:
> > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_8BIT;
> > - break;
> > - case 10:
> > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_10BIT;
> > - break;
> > - case 12:
> > - cfg |= CSI_IF_CFG_IF_DATA_WIDTH_12BIT;
> > - break;
> > - case 16: /* No need to configure DATA_WIDTH for 16bit */
> > - break;
> > - default:
> > - dev_warn(sdev->dev, "Unsupported bus width: %u\n", bus_width);
> > - break;
> > - }
> > -
>
> Is there any reason to move it around?
The main reason is cosmetics: input_parallel is introduced to match the already
existing input_interlaced variable, so it made sense to me to have both of these
conditionals one after the other instead of spreading them randomly.
I can mention this in the commit log if you prefer.
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027093119.GD168350%40aptenodytes.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH 03/14] media: sun6i-csi: Support an optional dedicated memory pool
Hi,
On Mon 26 Oct 20, 16:41, Maxime Ripard wrote:
> On Fri, Oct 23, 2020 at 07:45:35PM +0200, Paul Kocialkowski wrote:
> > This allows selecting a dedicated CMA memory pool (specified via
> > device-tree) instead of the default one.
> >
> > Signed-off-by: Paul Kocialkowski
>
> Why would that be needed?
Sorry for the confusion, this is indeed unrelated to the current series and
it is not needed for MIPI CSI-2 support.
However, I think it's a worthwhile addition to the driver.
I will take it out of the series and re-submit it separately then.
> > ---
> > drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > index 28e89340fed9..5d2389a5cd17 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -849,6 +850,12 @@ static int sun6i_csi_resource_request(struct
> > sun6i_csi_dev *sdev,
> > return PTR_ERR(sdev->regmap);
> > }
> >
> > + ret = of_reserved_mem_device_init(>dev);
> > + if (ret && ret != -ENODEV) {
> > + dev_err(>dev, "Unable to init reserved memory\n");
> > + return ret;
> > + }
> > +
> > sdev->clk_mod = devm_clk_get(>dev, "mod");
>
> If that clk_get or any subsequent function fail you'll end up leaking
> whatever the initialization of the reserved memory has allocated
Right, there's a missing of_reserved_mem_device_release in the error path here.
Thanks!
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20201027092638.GC168350%40aptenodytes.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH 02/14] phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI CSI-2
Hi,
On Mon 26 Oct 20, 16:38, Maxime Ripard wrote:
> On Fri, Oct 23, 2020 at 07:45:34PM +0200, Paul Kocialkowski wrote:
> > The Allwinner A31 D-PHY supports both Rx and Tx modes. While the latter
> > is already supported and used for MIPI DSI this adds support for the
> > former, to be used with MIPI CSI-2.
> >
> > This implementation is inspired by the Allwinner BSP implementation.
>
> Mentionning which BSP you took this from would be helpful
Sure! It's from the Github repo linked from https://linux-sunxi.org/V3s.
Would you like that I mention this URL explicitly or would it be enough to
mention "Allwinner's V3s Linux SDK" as they seem to call it?
> > Signed-off-by: Paul Kocialkowski
> > ---
> > drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 164 +++-
> > 1 file changed, 160 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> > b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> > index 1fa761ba6cbb..8bcd4bb79f60 100644
> > --- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> > +++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> > @@ -24,6 +24,14 @@
> > #define SUN6I_DPHY_TX_CTL_REG 0x04
> > #define SUN6I_DPHY_TX_CTL_HS_TX_CLK_CONT BIT(28)
> >
> > +#define SUN6I_DPHY_RX_CTL_REG 0x08
> > +#define SUN6I_DPHY_RX_CTL_EN_DBC BIT(31)
> > +#define SUN6I_DPHY_RX_CTL_RX_CLK_FORCE BIT(24)
> > +#define SUN6I_DPHY_RX_CTL_RX_D3_FORCE BIT(23)
> > +#define SUN6I_DPHY_RX_CTL_RX_D2_FORCE BIT(22)
> > +#define SUN6I_DPHY_RX_CTL_RX_D1_FORCE BIT(21)
> > +#define SUN6I_DPHY_RX_CTL_RX_D0_FORCE BIT(20)
> > +
>
> It's hard to tell from the diff, but it looks like you aligned the
> BIT(..) with the register?
That's correct, yes.
> If so, you should follow the what the rest of this driver (ie, one more
> indentation for register values).
I'll fix it in the next revision!
> > #define SUN6I_DPHY_TX_TIME0_REG0x10
> > #define SUN6I_DPHY_TX_TIME0_HS_TRAIL(n)(((n) & 0xff) << 24)
> > #define SUN6I_DPHY_TX_TIME0_HS_PREPARE(n) (((n) & 0xff) << 16)
> > @@ -44,12 +52,29 @@
> > #define SUN6I_DPHY_TX_TIME4_HS_TX_ANA1(n) (((n) & 0xff) << 8)
> > #define SUN6I_DPHY_TX_TIME4_HS_TX_ANA0(n) ((n) & 0xff)
> >
> > +#define SUN6I_DPHY_RX_TIME0_REG0x30
> > +#define SUN6I_DPHY_RX_TIME0_HS_RX_SYNC(n) (((n) & 0xff) << 24)
> > +#define SUN6I_DPHY_RX_TIME0_HS_RX_CLK_MISS(n) (((n) & 0xff) << 16)
> > +#define SUN6I_DPHY_RX_TIME0_LP_RX(n) (((n) & 0xff) << 8)
> > +
> > +#define SUN6I_DPHY_RX_TIME1_REG0x34
> > +#define SUN6I_DPHY_RX_TIME1_RX_DLY(n) (((n) & 0xfff) << 20)
> > +#define SUN6I_DPHY_RX_TIME1_LP_RX_ULPS_WP(n) ((n) & 0xf)
> > +
> > +#define SUN6I_DPHY_RX_TIME2_REG0x38
> > +#define SUN6I_DPHY_RX_TIME2_HS_RX_ANA1(n) (((n) & 0xff) << 8)
> > +#define SUN6I_DPHY_RX_TIME2_HS_RX_ANA0(n) ((n) & 0xff)
> > +
> > +#define SUN6I_DPHY_RX_TIME3_REG0x40
> > +#define SUN6I_DPHY_RX_TIME3_LPRST_DLY(n) (((n) & 0x) << 16)
> > +
> > #define SUN6I_DPHY_ANA0_REG0x4c
> > #define SUN6I_DPHY_ANA0_REG_PWSBIT(31)
> > #define SUN6I_DPHY_ANA0_REG_DMPC BIT(28)
> > #define SUN6I_DPHY_ANA0_REG_DMPD(n)(((n) & 0xf) << 24)
> > #define SUN6I_DPHY_ANA0_REG_SLV(n) (((n) & 7) << 12)
> > #define SUN6I_DPHY_ANA0_REG_DEN(n) (((n) & 0xf) << 8)
> > +#define SUN6I_DPHY_ANA0_REG_SFB(n) (((n) & 3) << 2)
> >
> > #define SUN6I_DPHY_ANA1_REG0x50
> > #define SUN6I_DPHY_ANA1_REG_VTTMODEBIT(31)
> > @@ -92,6 +117,8 @@ struct sun6i_dphy {
> >
> > struct phy *phy;
> > struct phy_configure_opts_mipi_dphy config;
> > +
> > + int submode;
> > };
> >
> > static int sun6i_dphy_init(struct phy *phy)
> > @@ -105,6 +132,18 @@ static int sun6i_dphy_init(struct phy *phy)
> > return 0;
> > }
> >
> > +static int sun6i_dphy_set_mode(struct phy *phy, enum phy_mode mode, int
> > submode)
> > +{
> > + struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + if (mode != PHY_MODE_MIPI_DPHY)
> > + return -EINVAL;
> > +
> > + dphy->submode = submode;
> > +
> > + return 0;
> > +}
> > +
> > static int sun6i_dphy_configure(struct phy *phy, union phy_configure_opts
> > *opts)
> > {
> > struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> > @@ -119,9 +158,8 @@ static int sun6i_dphy_configure(struct phy *phy, union
> > phy_configure_opts *opts)
> > return 0;
> > }
> >
> > -static int sun6i_dphy_power_on(struct phy *phy)
> > +static int sun6i_dphy_tx_power_on(struct sun6i_dphy *dphy)
> > {
> > - struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> > u8 lanes_mask = GENMASK(dphy->config.lanes - 1, 0);
> >
> > regmap_write(dphy->regs, SUN6I_DPHY_TX_CTL_REG,
> > @@ -211,12 +249,129 @@ static int sun6i_dphy_power_on(struct phy
