Re: [RFC v3 1/2] pinctrl: core: Add pinctrl_mux_gpio_request_enable
On Fri, Mar 1, 2019 at 2:27 PM Geert Uytterhoeven wrote: > /me gives v3 a try on Koelsch, Salvator-XS, and Ebisu-4D: > > BUG: sleeping function called from invalid context > > for mmc, adv7511, gpio-keys, and Ethernet PHY. This is the usual problem when you call back from any of the irqchip callbacks: almost all of them except request/release resources are called under a spinlock. The problem is creeping up in a lot of places, and I can't really solve that from the GPIO side. See for example this regression that I have no idea what to do with: https://marc.info/?l=linux-kernel&m=154349829407463&w=2 Yours, Linus Walleij
Re: [PATCH v8 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hi Sergei,
On Thu, Mar 7, 2019 at 6:50 PM Sergei Shtylyov
wrote:
> On 01/28/2019 09:49 AM, Mason Yang wrote:
> > Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
> >
> > Signed-off-by: Mason Yang
> > Signed-off-by: Sergei Shtylyov
> [...]
> > diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
> > new file mode 100644
> > index 000..ea12017
> > --- /dev/null
> > +++ b/drivers/spi/spi-renesas-rpc.c
> > @@ -0,0 +1,804 @@
> [...]
> > +static void rpc_spi_hw_init(struct rpc_spi *rpc)
> > +{
> > + //
> > + // NOTE: The 0x260 are undocumented bits, but they must be set.
> > + // RPC_PHYCNT_STRTIM is strobe timing adjustment bit,
> > + // 0x0 : the delay is biggest,
> > + // 0x1 : the delay is 2nd biggest,
> > + // On H3 ES1.x, the value should be 0, while on others,
> > + // the value should be 6.
> > + //
> > + regmap_write(rpc->regmap, RPC_PHYCNT, RPC_PHYCNT_CAL |
> > + RPC_PHYCNT_STRTIM(6) | 0x260);
> > +
> > + //
> > + // NOTE: The 0x1511144 are undocumented bits, but they must be set
> > + // for RPC_PHYOFFSET1.
> > + // The 0x31 are undocumented bits, but they must be set
> > + // for RPC_PHYOFFSET2.
> > + //
> > + regmap_write(rpc->regmap, RPC_PHYOFFSET1, RPC_PHYOFFSET1_DDRTMG(3) |
> > + 0x1511144);
> > + regmap_write(rpc->regmap, RPC_PHYOFFSET2, 0x31 |
> > + RPC_PHYOFFSET2_OCTTMG(4));
> > + regmap_write(rpc->regmap, RPC_SSLDR, RPC_SSLDR_SPNDL(7) |
> > + RPC_SSLDR_SLNDL(7) | RPC_SSLDR_SCKDL(7));
> > + regmap_write(rpc->regmap, RPC_CMNCR, RPC_CMNCR_MD | RPC_CMNCR_SFDE |
> > + RPC_CMNCR_MOIIO_HIZ | RPC_CMNCR_IOFV_HIZ |
> > + RPC_CMNCR_BSZ(0));
> > +}
>
>We clearly need runtime PM get/put() calls around this code. Otherwise,
> we're dependant on U-Boot leaving the clocks enabled...
Even that would be futile, as the common clock framework disables all
unused clocks at late boot time.
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] media: adv748x: Don't disable CSI-2 on link_setup
Hi Jacopo,
On Thu, Mar 07, 2019 at 11:35:11AM +0100, Jacopo Mondi wrote:
> On Wed, Mar 06, 2019 at 09:15:21PM +0200, Laurent Pinchart wrote:
> > On Wed, Mar 06, 2019 at 12:26:59PM +0100, Jacopo Mondi wrote:
> >> When both the media links between AFE and HDMI and the two TX CSI-2 outputs
> >> gets disabled, the routing register ADV748X_IO_10 gets zeroed causing both
> >> TXA and TXB output to get disabled.
> >>
> >> This causes some HDMI transmitters to stop working after both AFE and
> >> HDMI links are disabled.
> >
> > Could you elaborate on why this would be the case ? By HDMI transmitter,
> > I assume you mean the device connected to the HDMI input of the ADV748x.
> > Why makes it fail (and how ?) when the TXA and TXB are both disabled ?
>
> I know, it's weird, the HDMI transmitter is connected to the HDMI
> input of adv748x and should not be bothered by CSI-2 outputs
> enablement/disablement.
>
> BUT, when I developed the initial adv748x AFE->TXA patches I was
> testing HDMI capture using a laptop, and things were smooth.
>
> I recently started using a chrome cast device I found in some drawer
> to test HDMI, as with it I don't need to go through xrandr as I had to
> do when using a laptop for testing, but it seems the two behaves differently.
>
> Failures are of different types: from detecting a non-realisting
> resolution from the HDMI subdevice, and then messing up the pipeline
> configuration, to capture operations apparently completing properly
> but resulting in mangled images.
>
> Do not deactivate the CSI-2 ouputs seems to fix the issue for the
> Chromecast, and still work when capturing from laptop. There might be
> something I am missing about HDMI maybe, but the patch not just fixes
> the issue for me, but it might make sense on its own as disabling the
> TXes might trigger some internal power saving state, or simply mess up
> the HDMI link.
I think this needs more investigation. It feels to me that you're
working around an issue by chance, and it will come back to bite us
later :-(
> As disabling both TXes usually happens at media link reset time, just
> before enabling one of them (or both), going through a full disable
> makes little sense, even more if it triggers any sort of malfunctioning.
>
> Does this make sense to you?
It also doesn't make too much sense to keep them both enabled when they
don't need to be :-) You'll end up consuming more power.
> >> Fix this by preventing writing 0 to
> >> ADV748X_IO_10 register, which gets only updated when links are enabled
> >> again.
> >>
> >> Fixes: 9423ca350df7 ("media: adv748x: Implement TX link_setup callback")
> >> Signed-off-by: Jacopo Mondi
> >> ---
> >> The issue presents itself only on some HDMI transmitters, and went
> >> unnoticed
> >> during the development of:
> >> "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> >>
> >> Patch intended to be applied on top of latest media-master, where the
> >> "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> >> series is applied.
> >>
> >> The patch reports a "Fixes" tag, but should actually be merged with the
> >> above
> >> mentioned series.
> >>
> >> ---
> >> drivers/media/i2c/adv748x/adv748x-core.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
> >> b/drivers/media/i2c/adv748x/adv748x-core.c
> >> index f57cd77a32fa..0e5a75eb6d75 100644
> >> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> >> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> >> @@ -354,6 +354,9 @@ static int adv748x_link_setup(struct media_entity
> >> *entity,
> >>
> >>tx->src = enable ? rsd : NULL;
> >>
> >> + if (!enable)
> >> + return 0;
> >> +
> >>if (state->afe.tx) {
> >>/* AFE Requires TXA enabled, even when output to TXB */
> >>io10 |= ADV748X_IO_10_CSI4_EN;
--
Regards,
Laurent Pinchart
Re: [PATCH] arm64: dts: renesas: r8a77990: Remove invalid compatible value for CSI40
On Wed, Mar 06, 2019 at 06:45:56PM +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
>
> On Wed, Mar 6, 2019 at 4:14 PM Niklas Söderlund
> wrote:
> > The compatible value renesas,rcar-gen3-csi2 was used while prototyping
> > the R-Car CSI-2 driver but was removed before the driver was merged.
> > Remove the only occurrence of the compatible value which manage to make
>
> Nah, there's a second one, in r8a774c0.dtsi ;-)
>
> > it upstream.
> >
> > Signed-off-by: Niklas Söderlund
>
> Reviewed-by: Geert Uytterhoeven
Thanks,
should this patch have:
Fixes: ec70407ae7d7 ("arm64: dts: renesas: r8a77990: Add VIN and CSI-2 device
nodes")
Re: [PATCH] arm64: dts: renesas: r8a774c0-cat874: Add RWDT support
On Wed, Mar 06, 2019 at 11:52:03AM +, Chris Paterson wrote:
>
> > From: Fabrizio Castro
> > Sent: 06 March 2019 11:30
> >
> > Enable RWDT and use 60 seconds as default timeout.
> >
> > Signed-off-by: Fabrizio Castro
> Reviewed-by: Chris Paterson
Thanks, applied for v5.2.
> Kind regards, Chris
>
> > ---
> > arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > index 96ee0d2c..49ad3a5 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> > @@ -84,6 +84,11 @@
> > };
> > };
> >
> > +&rwdt {
> > + timeout-sec = <60>;
> > + status = "okay";
> > +};
> > +
> > &scif2 {
> > pinctrl-0 = <&scif2_pins>;
> > pinctrl-names = "default";
> > --
> > 2.7.4
>
Re: [PATCH v2 08/15] ARM: shmobile: fix a leaked reference by adding missing of_node_put
On Tue, Mar 05, 2019 at 07:33:59PM +0800, Wen Yang wrote:
> The call to of_get_next_child returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./arch/arm/mach-shmobile/pm-rcar-gen2.c:77:2-8: ERROR: missing of_node_put;
> acquired a node pointer with refcount incremented on line 66, but without a
> corresponding object release within this function.
> ./arch/arm/mach-shmobile/pm-rcar-gen2.c:85:2-8: ERROR: missing of_node_put;
> acquired a node pointer with refcount incremented on line 66, but without a
> corresponding object release within this function.
> ./arch/arm/mach-shmobile/pm-rcar-gen2.c:90:2-8: ERROR: missing of_node_put;
> acquired a node pointer with refcount incremented on line 66, but without a
> corresponding object release within this function.
>
> Signed-off-by: Wen Yang
> Reviewed-by: Florian Fainelli
> Reviewed-by: Geert Uytterhoeven
> Cc: Simon Horman
> Cc: Magnus Damm
> Cc: Russell King
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-ker...@vger.kernel.org
> ---
> v2->v1: add a missing space between "adding" and "missing"
Thanks, I have this applied for inclusion in v5.2.
> arch/arm/mach-shmobile/pm-rcar-gen2.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-shmobile/pm-rcar-gen2.c
> b/arch/arm/mach-shmobile/pm-rcar-gen2.c
> index 8c2a205..e84599d 100644
> --- a/arch/arm/mach-shmobile/pm-rcar-gen2.c
> +++ b/arch/arm/mach-shmobile/pm-rcar-gen2.c
> @@ -72,6 +72,7 @@ void __init rcar_gen2_pm_init(void)
> }
>
> error = of_address_to_resource(np, 0, &res);
> + of_node_put(np);
> if (error) {
> pr_err("Failed to get smp-sram address: %d\n", error);
> return;
> --
> 2.9.5
>
Re: [PATCH] rcar-csi2: Allow configuring of video standard
Hi Niklas,
(CC'ing Sakari)
On Thu, Mar 07, 2019 at 01:38:20AM +0100, Niklas Söderlund wrote:
> On 2019-03-07 02:26:45 +0200, Laurent Pinchart wrote:
> > On Thu, Mar 07, 2019 at 01:22:36AM +0100, Niklas Söderlund wrote:
> >> On 2019-03-07 02:13:18 +0200, Laurent Pinchart wrote:
> >>> On Sat, Feb 16, 2019 at 11:57:58PM +0100, Niklas Söderlund wrote:
> Allow the hardware to to do proper field detection for interlaced field
> formats by implementing s_std() and g_std(). Depending on which video
> standard is selected the driver needs to setup the hardware to correctly
> identify fields.
> >>>
> >>> I don't think this belongs to the CSI-2 receiver. Standards are really
> >>> an analog concept, and should be handled by the analog front-end. At the
> >>> CSI-2 level there's no concept of analog standard anymore.
> >>
> >> I agree it should be handled by the analog front-end. This is patch just
> >> lets the user propagate the information in the pipeline. The driver
> >> could instead find its source subdevice in the media graph and ask which
> >> standard it's supplying.
> >>
> >> I wrestled a bit with this and went with this approach as it then works
> >> the same as with other format information, such as dimensions and pixel
> >> format. If the driver acquires the standard by itself why should it no
> >> the same for the format? I'm willing to change this but I would like to
> >> understand where the divider for format propagating in kernel and
> >> user-space is :-)
> >>
> >> Also what if there are subdevices between rcar-csi2 and the analog
> >> front-end which do not support the g_std operation?
> >
> > My point is that the analog standard shouldn't be propagated at all,
> > neither inside the kernel nor in userspace, as it is not applicable to
> > CSI-2.
>
> This is not related to CSI-2 if I understand it. It is related to the
> outputed field signal on the parallel output from CSI-2 facing the VINs.
> See chapter "25.4.5 FLD Signal" in the datasheet.
I would solely use the information contained in v4l2_mbus_framefmt. You
could use the frame height for instance to detect the type of standard.
> Later versions of the datasheet have also been updated to make it clear
> that FLD register should be set to 0 when dealing with none interlaced
> field formats.
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 33 +++--
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index f3099f3e536d808a..664d3784be2b9db9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -361,6 +361,7 @@ struct rcar_csi2 {
> struct v4l2_subdev *remote;
>
> struct v4l2_mbus_framefmt mf;
> +v4l2_std_id std;
>
> struct mutex lock;
> int stream_count;
> @@ -389,6 +390,22 @@ static void rcsi2_write(struct rcar_csi2 *priv,
> unsigned int reg, u32 data)
> iowrite32(data, priv->base + reg);
> }
>
> +static int rcsi2_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> +{
> +struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> +priv->std = std;
> +return 0;
> +}
> +
> +static int rcsi2_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
> +{
> +struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> +*std = priv->std;
> +return 0;
> +}
> +
> static void rcsi2_standby_mode(struct rcar_csi2 *priv, int on)
> {
> if (!on) {
> @@ -475,7 +492,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> unsigned int bpp)
> static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> {
> const struct rcar_csi2_format *format;
> -u32 phycnt, vcdt = 0, vcdt2 = 0;
> +u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
> unsigned int i;
> int mbps, ret;
>
> @@ -507,6 +524,15 @@ static int rcsi2_start_receiver(struct rcar_csi2
> *priv)
> vcdt2 |= vcdt_part << ((i % 2) * 16);
> }
>
> +if (priv->mf.field != V4L2_FIELD_NONE) {
> +fld = FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 |
> FLD_FLD_EN;
> +
> +if (priv->std & V4L2_STD_525_60)
> +fld |= FLD_FLD_NUM(2);
> +else
> +fld |= FLD_FLD_NUM(1);
> +}
> +
> phycnt = PHYCNT_ENABLECLK;
> phycnt |= (1 << priv->lanes) - 1;
>
> @@ -519,8 +545,7 @@ static int rcsi2_start_receiver(struct rcar_csi
Re: [PATCH] media: adv748x: Don't disable CSI-2 on link_setup
Hi Laurent,
On Fri, Mar 08, 2019 at 01:29:38PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Thu, Mar 07, 2019 at 11:35:11AM +0100, Jacopo Mondi wrote:
> > On Wed, Mar 06, 2019 at 09:15:21PM +0200, Laurent Pinchart wrote:
> > > On Wed, Mar 06, 2019 at 12:26:59PM +0100, Jacopo Mondi wrote:
> > >> When both the media links between AFE and HDMI and the two TX CSI-2
> > >> outputs
> > >> gets disabled, the routing register ADV748X_IO_10 gets zeroed causing
> > >> both
> > >> TXA and TXB output to get disabled.
> > >>
> > >> This causes some HDMI transmitters to stop working after both AFE and
> > >> HDMI links are disabled.
> > >
> > > Could you elaborate on why this would be the case ? By HDMI transmitter,
> > > I assume you mean the device connected to the HDMI input of the ADV748x.
> > > Why makes it fail (and how ?) when the TXA and TXB are both disabled ?
> >
> > I know, it's weird, the HDMI transmitter is connected to the HDMI
> > input of adv748x and should not be bothered by CSI-2 outputs
> > enablement/disablement.
> >
> > BUT, when I developed the initial adv748x AFE->TXA patches I was
> > testing HDMI capture using a laptop, and things were smooth.
> >
> > I recently started using a chrome cast device I found in some drawer
> > to test HDMI, as with it I don't need to go through xrandr as I had to
> > do when using a laptop for testing, but it seems the two behaves
> > differently.
> >
> > Failures are of different types: from detecting a non-realisting
> > resolution from the HDMI subdevice, and then messing up the pipeline
> > configuration, to capture operations apparently completing properly
> > but resulting in mangled images.
> >
> > Do not deactivate the CSI-2 ouputs seems to fix the issue for the
> > Chromecast, and still work when capturing from laptop. There might be
> > something I am missing about HDMI maybe, but the patch not just fixes
> > the issue for me, but it might make sense on its own as disabling the
> > TXes might trigger some internal power saving state, or simply mess up
> > the HDMI link.
>
> I think this needs more investigation. It feels to me that you're
> working around an issue by chance, and it will come back to bite us
> later :-(
>
I'm sorry I really can't tell what's happening, and why it is
happening on that specific device, which I cannot debug for sure.
Ian suggested a possible cause, but I cannot tell due to my
HDMI-ignorance.
> > As disabling both TXes usually happens at media link reset time, just
> > before enabling one of them (or both), going through a full disable
> > makes little sense, even more if it triggers any sort of malfunctioning.
> >
> > Does this make sense to you?
>
> It also doesn't make too much sense to keep them both enabled when they
> don't need to be :-) You'll end up consuming more power.
>
They've alwyas been up before introduction of dynamic routing, provided
something is connected to the TX source pad in DT.
https://elixir.bootlin.com/linux/latest/source/drivers/media/i2c/adv748x/adv748x-core.c#L489
Power saving wise, we're not doing worse than before, and if that's a
concern, it should be identified first why the CSI-2 Tx PLL never gets
turned off:
https://elixir.bootlin.com/linux/latest/source/drivers/media/i2c/adv748x/adv748x-core.c#L269
See "mipi_pll_en, CSI-TXA Map, Address 0xDA[0]" registrer description.
The two issues might be actually connected, I tried fixing this in the past,
but frame capture broke, and I didn't have time to investigate
fruther.
To sum up, this patch solves an issue on some devices, it does not
perform worse than what we had from a power consumption perspective,
but I agree it might work around some deeper issues it might be worth
chasing.
If I got your NAK on this, I'll keep carrying it in my tree when
testing with that device.
Thanks
j
> > >> Fix this by preventing writing 0 to
> > >> ADV748X_IO_10 register, which gets only updated when links are enabled
> > >> again.
> > >>
> > >> Fixes: 9423ca350df7 ("media: adv748x: Implement TX link_setup callback")
> > >> Signed-off-by: Jacopo Mondi
> > >> ---
> > >> The issue presents itself only on some HDMI transmitters, and went
> > >> unnoticed
> > >> during the development of:
> > >> "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> > >>
> > >> Patch intended to be applied on top of latest media-master, where the
> > >> "[PATCH v3 0/6] media: adv748x: Implement dynamic routing support"
> > >> series is applied.
> > >>
> > >> The patch reports a "Fixes" tag, but should actually be merged with the
> > >> above
> > >> mentioned series.
> > >>
> > >> ---
> > >> drivers/media/i2c/adv748x/adv748x-core.c | 3 +++
> > >> 1 file changed, 3 insertions(+)
> > >>
> > >> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
> > >> b/drivers/media/i2c/adv748x/adv748x-core.c
> > >> index f57cd77a32fa..0e5a75eb6d75 100644
> > >> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> > >> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
>
Re: [PATCH v3 00/31] v4l: add support for multiplexed streams
Hi Sakari and Niklas, On Thu, Mar 07, 2019 at 11:47:26AM +0200, Sakari Ailus wrote: > Hi Jacopo, > > On Tue, Mar 05, 2019 at 07:51:19PM +0100, Jacopo Mondi wrote: > > Hello, > >third version of multiplexed stream support patch series. > > > > V2 sent by Niklas is available at: > > https://patchwork.kernel.org/cover/10573817/ > > > > As per v2, most of the core patches are work from Sakari and Laurent, with > > Niklas' support on top for adv748x and rcar-csi2. > > > > The use case of the series remains the same: support for virtual channel > > selection implemented on R-Car Gen3 and adv748x. Quoting the v2 cover > > letter: > > > > --- > > I have added driver support for the devices used on the Renesas Gen3 > > platforms, a ADV7482 connected to the R-Car CSI-2 receiver. With these > > changes I can control which of the analog inputs of the ADV7482 the > > video source is captured from and on which CSI-2 virtual channel the > > video is transmitted on to the R-Car CSI-2 receiver. > > > > The series adds two new subdev IOCTLs [GS]_ROUTING which allows > > user-space to get and set routes inside a subdevice. I have added RFC > > support for these to v4l-utils [2] which can be used to test this > > series, example: > > > > Check the internal routing of the adv748x csi-2 transmitter: > > v4l2-ctl -d /dev/v4l-subdev24 --get-routing > > 0/0 -> 1/0 [ENABLED] > > 0/0 -> 1/1 [] > > 0/0 -> 1/2 [] > > 0/0 -> 1/3 [] > > > > > > Select that video should be outputed on VC 2 and check the result: > > $ v4l2-ctl -d /dev/v4l-subdev24 --set-routing '0/0 -> 1/2 [1]' > > > > $ v4l2-ctl -d /dev/v4l-subdev24 --get-routing > > 0/0 -> 1/0 [] > > 0/0 -> 1/1 [] > > 0/0 -> 1/2 [ENABLED] > > 0/0 -> 1/3 [] > > --- > > > > Below is reported the media graph of the system used for testing [1]. > > > > v4l2-ctl patches to handle the newly introduced IOCTLs are available from > > Niklas' repository at: > > git://git.ragnatech.se/v4l-utils routing > > Could you send the v4l2-ctl patches out as well, please? > Niklas sent them on late 2017... time flies :) https://patchwork.kernel.org/patch/10113189/ Would you like to have them re-sent? Thanks j > -- > Sakari Ailus > sakari.ai...@linux.intel.com signature.asc Description: PGP signature
Re: [PATCH v3 19/31] media: Documentation: Add GS_ROUTING documentation
Hi Sakari, thanks for the review On Thu, Mar 07, 2019 at 05:19:29PM +0200, Sakari Ailus wrote: > Hi Jacopo, > > Thanks for writing the documentation for this! > > The text is nice and informative; I have a few suggestions below. > > On Tue, Mar 05, 2019 at 07:51:38PM +0100, Jacopo Mondi wrote: > > Add documentation for VIDIOC_SUBDEV_G/S_ROUTING ioctl and add > > description of multiplexed media pads and internal routing to the > > V4L2-subdev documentation section. > > > > Signed-off-by: Jacopo Mondi > > --- > > Documentation/media/uapi/v4l/dev-subdev.rst | 90 +++ > > Documentation/media/uapi/v4l/user-func.rst| 1 + > > .../uapi/v4l/vidioc-subdev-g-routing.rst | 142 ++ > > 3 files changed, 233 insertions(+) > > create mode 100644 Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst > > > > diff --git a/Documentation/media/uapi/v4l/dev-subdev.rst > > b/Documentation/media/uapi/v4l/dev-subdev.rst > > index 2c2768c7343b..b9fbb5d2caec 100644 > > --- a/Documentation/media/uapi/v4l/dev-subdev.rst > > +++ b/Documentation/media/uapi/v4l/dev-subdev.rst > > @@ -36,6 +36,8 @@ will feature a character device node on which ioctls can > > be called to > > > > - negotiate image formats on individual pads > > > > +- inspect and modify internal data routing between pads of the same entity > > + > > Sub-device character device nodes, conventionally named > > ``/dev/v4l-subdev*``, use major number 81. > > > > @@ -461,3 +463,91 @@ source pads. > > :maxdepth: 1 > > > > subdev-formats > > + > > + > > +Multiplexed media pads and internal routing > > +--- > > + > > +Subdevice drivers might expose the internal connections between media pads > > of an > > s/might/may/ > > > +entity by providing a routing table that applications can inspect and > > manipulate > > s/providing/exposing/ > Ack on both the above suggestions > > +to change the internal routing between sink and source pads' data > > connection > > +endpoints. A routing table is described by a struct > > +:c:type:`v4l2_subdev_routing`, which contains ``num_routes`` route > > entries, each > > +one represented by a struct :c:type:`v4l2_subdev_route`. > > + > > +Data routes do not just connect one pad to another in an entity, but they > > refer > > +instead to the ``streams`` a media pad provides. Streams are data > > connection > > +endpoints in a media pad. Multiplexed media pads expose multiple > > ``streams`` > > +which represent, when the underlying hardware technology allows that, > > logical > > +data streams transported over a single physical media bus. > > How about s/streams/flows/ for this instance? > Agreed, there are too many "streams" already in that paragraph > > + > > +One of the most notable examples of logical stream multiplexing techniques > > is > > s/One of the most notable examples/A noteworthy example/ > > > +represented by the data interleaving mechanism implemented by mean of > > Virtual > > +Channels identifiers as defined by the MIPI CSI-2 media bus > > specifications. A > > s/identifiers // > Ack on both the above suggestions > > +subdevice that implements support for Virtual Channel data interleaving > > might > > +expose up to 4 data ``streams``, one for each available Virtual Channel, > > on the > > +source media pad that represents a CSI-2 connection endpoint. > > + > > +Routes are defined as potential data connections between a ``(sink_pad, > > +sink_stream)`` pair and a ``(source_pad, source_stream)`` one, where > > +``sink_pad`` and ``source_pad`` are the indexes of two media pads part of > > the > > +same media entity, and ``sink_stream`` and ``source_stream`` are the > > identifiers > > +of the data streams to be connected in the media pads. Media pads that do > > not > > +support data multiplexing expose a single stream, usually with identifier > > 0. > > + > > +Routes are reported to applications in a routing table which can be > > +inspected and manipulated using the :ref:`routing > > ` > > +ioctls. > > + > > +Routes can be activated and deactivated by setting or clearing the > > +V4L2_SUBDEV_ROUTE_FL_ACTIVE flag in the ``flags`` field of struct > > +:c:type:`v4l2_subdev_route`. > > + > > +Subdev driver might create routes that cannot be modified by applications. > > Such > > s/S/A s/ Or "Subdevice drivers" ? > > > +routes are identified by the presence of the V4L2_SUBDEV_ROUTE_FL_IMMUTABLE > > +flag in the ``flags`` field of struct :c:type:`v4l2_subdev_route`. > > + > > +As an example, the routing table of a source pad which supports two logical > > +video streams and can be connected to two sink pads is here below > > described. > > + > > +.. flat-table:: > > +:widths: 1 2 1 > > + > > +* - Pad Index > > + - Function > > + - Number of streams > > +* - 0 > > + - SINK > > + - 1 > > +* - 1 > > + - SINK > > + - 1 > > +* - 2 > > + - SOURCE > > +
Re: [PATCH] ARM: dts: r8a7792: blanche: Add IIC3 and DA9063 PMIC node
On Mon, Mar 04, 2019 at 08:59:58PM +0100, marek.va...@gmail.com wrote: > From: Marek Vasut > > Add IIC3 node to R8A7792 SoC device tree and a DA9063 PMIC node > to V2H Blanche board device tree. > > Signed-off-by: Marek Vasut > Cc: Geert Uytterhoeven > Cc: Simon Horman > Cc: linux-renesas-soc@vger.kernel.org > To: linux-arm-ker...@lists.infradead.org > --- > NOTE: R8A7792 Blanche does not share the PMIC IRQ line with other PMICs, > hence the regulator-quirk-rcar-gen2.c does not need a new entry. > --- > arch/arm/boot/dts/r8a7792-blanche.dts | 20 > arch/arm/boot/dts/r8a7792.dtsi| 18 ++ > 2 files changed, 38 insertions(+) Thanks, applied for v5.2.
Re: [PATCH] ARM: dts: alt: Add DA9063 PMIC node
On Mon, Mar 04, 2019 at 12:13:49PM +0100, Wolfram Sang wrote:
> On Mon, Mar 04, 2019 at 11:59:48AM +0100, Marek Vasut wrote:
> > On 3/4/19 11:57 AM, Wolfram Sang wrote:
> > >
> > >>> Add DA9063 PMIC node to the I2C bus.
> > >>>
> > >>> Signed-off-by: Marek Vasut
> > >>
> > >> Thanks for your patch!
> > >>
> > >> Reviewed-by: Geert Uytterhoeven
> > >
> > > Thanks for CCing me, I didn't get and missed the original patch.
> > >
> > >>> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> > >>> @@ -377,6 +377,27 @@
> > >>> pinctrl-names = "i2c-exio4";
> > >>> };
> > >>>
> > >>> +&i2c7 {
> > >>> + status = "okay";
> > >>> + clock-frequency = <10>;
> > >>
> > >> According to the DA9063 datasheet, the PMIC supports up to 400 kHz.
> > >> It looks like there are no other devices on the bus.
> > >>
> > >> Wolfram, what's your stance on this?
> > >
> > > Yes, please. I thought this was standard on Gen2 IIC_DVFS busses, but
> > > seems not. Probably it slipped through the cracks.
> >
> > Do we want to update all of the other boards too ?
> >
> > I'd be cautious about the DVFS I2C, running faster while talking to the
> > PMIC and reading/writing a few registers brings little benefit, while
> > the signal integrity might be impacted.
>
> Can be argued. I think if we would apply some testing on top of this
> change, we will find that it will work. But we would in deed need this
> testing as verification, and this is not our top priority project. But
> if someone feels like running some i2c dumping over night, well,
> why not..
Thanks,
I've applied this patch for v5.2.
We can do a sweep of frequency updates as a follow-up if we decide
that is the right way to go.
Re: [PATCH] ARM: dts: alt: Add DA9063 PMIC node
On 3/8/19 2:40 PM, Simon Horman wrote:
> On Mon, Mar 04, 2019 at 12:13:49PM +0100, Wolfram Sang wrote:
>> On Mon, Mar 04, 2019 at 11:59:48AM +0100, Marek Vasut wrote:
>>> On 3/4/19 11:57 AM, Wolfram Sang wrote:
>> Add DA9063 PMIC node to the I2C bus.
>>
>> Signed-off-by: Marek Vasut
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven
Thanks for CCing me, I didn't get and missed the original patch.
>> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
>> @@ -377,6 +377,27 @@
>> pinctrl-names = "i2c-exio4";
>> };
>>
>> +&i2c7 {
>> + status = "okay";
>> + clock-frequency = <10>;
>
> According to the DA9063 datasheet, the PMIC supports up to 400 kHz.
> It looks like there are no other devices on the bus.
>
> Wolfram, what's your stance on this?
Yes, please. I thought this was standard on Gen2 IIC_DVFS busses, but
seems not. Probably it slipped through the cracks.
>>>
>>> Do we want to update all of the other boards too ?
>>>
>>> I'd be cautious about the DVFS I2C, running faster while talking to the
>>> PMIC and reading/writing a few registers brings little benefit, while
>>> the signal integrity might be impacted.
>>
>> Can be argued. I think if we would apply some testing on top of this
>> change, we will find that it will work. But we would in deed need this
>> testing as verification, and this is not our top priority project. But
>> if someone feels like running some i2c dumping over night, well,
>> why not..
>
> Thanks,
>
> I've applied this patch for v5.2.
> We can do a sweep of frequency updates as a follow-up if we decide
> that is the right way to go.
Do we really want to speed up the PMIC I2C bus ? What are the pros
(none?) and cons (might be unstable) ?
--
Best regards,
Marek Vasut
Re: [PATCH] rcar-csi2: Allow configuring of video standard
On 2/16/19 11:57 PM, Niklas Söderlund wrote:
> Allow the hardware to to do proper field detection for interlaced field
> formats by implementing s_std() and g_std(). Depending on which video
> standard is selected the driver needs to setup the hardware to correctly
> identify fields.
>
> Later versions of the datasheet have also been updated to make it clear
> that FLD register should be set to 0 when dealing with none interlaced
> field formats.
Nacked-by: Hans Verkuil
The G/S_STD and QUERYSTD ioctls are specific for SDTV video receivers
(composite, S-Video, analog tuner) and can't be used for CSI devices.
struct v4l2_mbus_framefmt already has a 'field' value that is explicit
about the field ordering (TB vs BT) or the field ordering can be deduced
from the frame height (FIELD_INTERLACED).
Regards,
Hans
>
> Signed-off-by: Niklas Söderlund
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 33 +++--
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index f3099f3e536d808a..664d3784be2b9db9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -361,6 +361,7 @@ struct rcar_csi2 {
> struct v4l2_subdev *remote;
>
> struct v4l2_mbus_framefmt mf;
> + v4l2_std_id std;
>
> struct mutex lock;
> int stream_count;
> @@ -389,6 +390,22 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned
> int reg, u32 data)
> iowrite32(data, priv->base + reg);
> }
>
> +static int rcsi2_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> +{
> + struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> + priv->std = std;
> + return 0;
> +}
> +
> +static int rcsi2_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
> +{
> + struct rcar_csi2 *priv = sd_to_csi2(sd);
> +
> + *std = priv->std;
> + return 0;
> +}
> +
> static void rcsi2_standby_mode(struct rcar_csi2 *priv, int on)
> {
> if (!on) {
> @@ -475,7 +492,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> unsigned int bpp)
> static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> {
> const struct rcar_csi2_format *format;
> - u32 phycnt, vcdt = 0, vcdt2 = 0;
> + u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
> unsigned int i;
> int mbps, ret;
>
> @@ -507,6 +524,15 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> vcdt2 |= vcdt_part << ((i % 2) * 16);
> }
>
> + if (priv->mf.field != V4L2_FIELD_NONE) {
> + fld = FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN;
> +
> + if (priv->std & V4L2_STD_525_60)
> + fld |= FLD_FLD_NUM(2);
> + else
> + fld |= FLD_FLD_NUM(1);
> + }
> +
> phycnt = PHYCNT_ENABLECLK;
> phycnt |= (1 << priv->lanes) - 1;
>
> @@ -519,8 +545,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> rcsi2_write(priv, PHTC_REG, 0);
>
> /* Configure */
> - rcsi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> - FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> + rcsi2_write(priv, FLD_REG, fld);
> rcsi2_write(priv, VCDT_REG, vcdt);
> if (vcdt2)
> rcsi2_write(priv, VCDT2_REG, vcdt2);
> @@ -662,6 +687,8 @@ static int rcsi2_get_pad_format(struct v4l2_subdev *sd,
> }
>
> static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
> + .s_std = rcsi2_s_std,
> + .g_std = rcsi2_g_std,
> .s_stream = rcsi2_s_stream,
> };
>
>
Re: [PATCH v3 00/31] v4l: add support for multiplexed streams
On Fri, Mar 08, 2019 at 02:19:03PM +0100, Jacopo Mondi wrote: > Hi Sakari and Niklas, > > On Thu, Mar 07, 2019 at 11:47:26AM +0200, Sakari Ailus wrote: > > Hi Jacopo, > > > > On Tue, Mar 05, 2019 at 07:51:19PM +0100, Jacopo Mondi wrote: > > > Hello, > > >third version of multiplexed stream support patch series. > > > > > > V2 sent by Niklas is available at: > > > https://patchwork.kernel.org/cover/10573817/ > > > > > > As per v2, most of the core patches are work from Sakari and Laurent, with > > > Niklas' support on top for adv748x and rcar-csi2. > > > > > > The use case of the series remains the same: support for virtual channel > > > selection implemented on R-Car Gen3 and adv748x. Quoting the v2 cover > > > letter: > > > > > > --- > > > I have added driver support for the devices used on the Renesas Gen3 > > > platforms, a ADV7482 connected to the R-Car CSI-2 receiver. With these > > > changes I can control which of the analog inputs of the ADV7482 the > > > video source is captured from and on which CSI-2 virtual channel the > > > video is transmitted on to the R-Car CSI-2 receiver. > > > > > > The series adds two new subdev IOCTLs [GS]_ROUTING which allows > > > user-space to get and set routes inside a subdevice. I have added RFC > > > support for these to v4l-utils [2] which can be used to test this > > > series, example: > > > > > > Check the internal routing of the adv748x csi-2 transmitter: > > > v4l2-ctl -d /dev/v4l-subdev24 --get-routing > > > 0/0 -> 1/0 [ENABLED] > > > 0/0 -> 1/1 [] > > > 0/0 -> 1/2 [] > > > 0/0 -> 1/3 [] > > > > > > > > > Select that video should be outputed on VC 2 and check the result: > > > $ v4l2-ctl -d /dev/v4l-subdev24 --set-routing '0/0 -> 1/2 [1]' > > > > > > $ v4l2-ctl -d /dev/v4l-subdev24 --get-routing > > > 0/0 -> 1/0 [] > > > 0/0 -> 1/1 [] > > > 0/0 -> 1/2 [ENABLED] > > > 0/0 -> 1/3 [] > > > --- > > > > > > Below is reported the media graph of the system used for testing [1]. > > > > > > v4l2-ctl patches to handle the newly introduced IOCTLs are available from > > > Niklas' repository at: > > > git://git.ragnatech.se/v4l-utils routing > > > > Could you send the v4l2-ctl patches out as well, please? > > > > Niklas sent them on late 2017... time flies :) > https://patchwork.kernel.org/patch/10113189/ > > Would you like to have them re-sent? Oh, well, sure, if there are no changes, no need to resend. Thanks for the pointer. -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v3 19/31] media: Documentation: Add GS_ROUTING documentation
Hi Jacopo, On Fri, Mar 08, 2019 at 02:31:33PM +0100, Jacopo Mondi wrote: > Hi Sakari, >thanks for the review > > On Thu, Mar 07, 2019 at 05:19:29PM +0200, Sakari Ailus wrote: > > Hi Jacopo, > > > > Thanks for writing the documentation for this! > > > > The text is nice and informative; I have a few suggestions below. > > > > On Tue, Mar 05, 2019 at 07:51:38PM +0100, Jacopo Mondi wrote: > > > Add documentation for VIDIOC_SUBDEV_G/S_ROUTING ioctl and add > > > description of multiplexed media pads and internal routing to the > > > V4L2-subdev documentation section. > > > > > > Signed-off-by: Jacopo Mondi > > > --- > > > Documentation/media/uapi/v4l/dev-subdev.rst | 90 +++ > > > Documentation/media/uapi/v4l/user-func.rst| 1 + > > > .../uapi/v4l/vidioc-subdev-g-routing.rst | 142 ++ > > > 3 files changed, 233 insertions(+) > > > create mode 100644 > > > Documentation/media/uapi/v4l/vidioc-subdev-g-routing.rst > > > > > > diff --git a/Documentation/media/uapi/v4l/dev-subdev.rst > > > b/Documentation/media/uapi/v4l/dev-subdev.rst > > > index 2c2768c7343b..b9fbb5d2caec 100644 > > > --- a/Documentation/media/uapi/v4l/dev-subdev.rst > > > +++ b/Documentation/media/uapi/v4l/dev-subdev.rst > > > @@ -36,6 +36,8 @@ will feature a character device node on which ioctls > > > can be called to > > > > > > - negotiate image formats on individual pads > > > > > > +- inspect and modify internal data routing between pads of the same > > > entity > > > + > > > Sub-device character device nodes, conventionally named > > > ``/dev/v4l-subdev*``, use major number 81. > > > > > > @@ -461,3 +463,91 @@ source pads. > > > :maxdepth: 1 > > > > > > subdev-formats > > > + > > > + > > > +Multiplexed media pads and internal routing > > > +--- > > > + > > > +Subdevice drivers might expose the internal connections between media > > > pads of an > > > > s/might/may/ > > > > > +entity by providing a routing table that applications can inspect and > > > manipulate > > > > s/providing/exposing/ > > > > Ack on both the above suggestions > > > > +to change the internal routing between sink and source pads' data > > > connection > > > +endpoints. A routing table is described by a struct > > > +:c:type:`v4l2_subdev_routing`, which contains ``num_routes`` route > > > entries, each > > > +one represented by a struct :c:type:`v4l2_subdev_route`. > > > + > > > +Data routes do not just connect one pad to another in an entity, but > > > they refer > > > +instead to the ``streams`` a media pad provides. Streams are data > > > connection > > > +endpoints in a media pad. Multiplexed media pads expose multiple > > > ``streams`` > > > +which represent, when the underlying hardware technology allows that, > > > logical > > > +data streams transported over a single physical media bus. > > > > How about s/streams/flows/ for this instance? > > > > Agreed, there are too many "streams" already in that paragraph > > > > + > > > +One of the most notable examples of logical stream multiplexing > > > techniques is > > > > s/One of the most notable examples/A noteworthy example/ > > > > > +represented by the data interleaving mechanism implemented by mean of > > > Virtual > > > +Channels identifiers as defined by the MIPI CSI-2 media bus > > > specifications. A > > > > s/identifiers // > > > > Ack on both the above suggestions > > > > +subdevice that implements support for Virtual Channel data interleaving > > > might > > > +expose up to 4 data ``streams``, one for each available Virtual Channel, > > > on the > > > +source media pad that represents a CSI-2 connection endpoint. > > > + > > > +Routes are defined as potential data connections between a ``(sink_pad, > > > +sink_stream)`` pair and a ``(source_pad, source_stream)`` one, where > > > +``sink_pad`` and ``source_pad`` are the indexes of two media pads part > > > of the > > > +same media entity, and ``sink_stream`` and ``source_stream`` are the > > > identifiers > > > +of the data streams to be connected in the media pads. Media pads that > > > do not > > > +support data multiplexing expose a single stream, usually with > > > identifier 0. > > > + > > > +Routes are reported to applications in a routing table which can be > > > +inspected and manipulated using the :ref:`routing > > > ` > > > +ioctls. > > > + > > > +Routes can be activated and deactivated by setting or clearing the > > > +V4L2_SUBDEV_ROUTE_FL_ACTIVE flag in the ``flags`` field of struct > > > +:c:type:`v4l2_subdev_route`. > > > + > > > +Subdev driver might create routes that cannot be modified by > > > applications. Such > > > > s/S/A s/ > > Or "Subdevice drivers" ? I think singular is fine. And s/might/may/. > > > > > > +routes are identified by the presence of the > > > V4L2_SUBDEV_ROUTE_FL_IMMUTABLE > > > +flag in the ``flags`` field of struct :c:type:`v4l2_subdev_route`. > > > + > > > +As an example, the routing
Re: [PATCH] arm64: dts: renesas: r8a77990: Remove invalid compatible value for CSI40
Hi Simon,
On 2019-03-08 13:02:33 +0100, Simon Horman wrote:
> On Wed, Mar 06, 2019 at 06:45:56PM +0100, Geert Uytterhoeven wrote:
> > Hi Niklas,
> >
> > On Wed, Mar 6, 2019 at 4:14 PM Niklas Söderlund
> > wrote:
> > > The compatible value renesas,rcar-gen3-csi2 was used while prototyping
> > > the R-Car CSI-2 driver but was removed before the driver was merged.
> > > Remove the only occurrence of the compatible value which manage to make
> >
> > Nah, there's a second one, in r8a774c0.dtsi ;-)
> >
> > > it upstream.
> > >
> > > Signed-off-by: Niklas Söderlund
> >
> > Reviewed-by: Geert Uytterhoeven
>
> Thanks,
>
> should this patch have:
>
> Fixes: ec70407ae7d7 ("arm64: dts: renesas: r8a77990: Add VIN and CSI-2 device
> nodes")
Yes it should. Want me to send a v2 or can you add then when picking up
the patch?
--
Regards,
Niklas Söderlund
Re: [PATCH] ARM: dts: alt: Add DA9063 PMIC node
> Do we really want to speed up the PMIC I2C bus ? What are the pros > (none?) and cons (might be unstable) ? Then let's go down to 10kHz instead? Even more stable. signature.asc Description: PGP signature
Re: [PATCH] rcar-csi2: Allow configuring of video standard
Hi Hans,
Thanks for your feedback.
On 2019-03-08 15:12:15 +0100, Hans Verkuil wrote:
> On 2/16/19 11:57 PM, Niklas Söderlund wrote:
> > Allow the hardware to to do proper field detection for interlaced field
> > formats by implementing s_std() and g_std(). Depending on which video
> > standard is selected the driver needs to setup the hardware to correctly
> > identify fields.
> >
> > Later versions of the datasheet have also been updated to make it clear
> > that FLD register should be set to 0 when dealing with none interlaced
> > field formats.
>
> Nacked-by: Hans Verkuil
>
> The G/S_STD and QUERYSTD ioctls are specific for SDTV video receivers
> (composite, S-Video, analog tuner) and can't be used for CSI devices.
>
> struct v4l2_mbus_framefmt already has a 'field' value that is explicit
> about the field ordering (TB vs BT) or the field ordering can be deduced
> from the frame height (FIELD_INTERLACED).
I will drop this patch and write a new one using field and height as
suggested by you and Laurent. Thanks for the suggestion!
>
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Niklas Söderlund
> > ---
> > drivers/media/platform/rcar-vin/rcar-csi2.c | 33 +++--
> > 1 file changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index f3099f3e536d808a..664d3784be2b9db9 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -361,6 +361,7 @@ struct rcar_csi2 {
> > struct v4l2_subdev *remote;
> >
> > struct v4l2_mbus_framefmt mf;
> > + v4l2_std_id std;
> >
> > struct mutex lock;
> > int stream_count;
> > @@ -389,6 +390,22 @@ static void rcsi2_write(struct rcar_csi2 *priv,
> > unsigned int reg, u32 data)
> > iowrite32(data, priv->base + reg);
> > }
> >
> > +static int rcsi2_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> > +{
> > + struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > + priv->std = std;
> > + return 0;
> > +}
> > +
> > +static int rcsi2_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
> > +{
> > + struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > + *std = priv->std;
> > + return 0;
> > +}
> > +
> > static void rcsi2_standby_mode(struct rcar_csi2 *priv, int on)
> > {
> > if (!on) {
> > @@ -475,7 +492,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv,
> > unsigned int bpp)
> > static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > {
> > const struct rcar_csi2_format *format;
> > - u32 phycnt, vcdt = 0, vcdt2 = 0;
> > + u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
> > unsigned int i;
> > int mbps, ret;
> >
> > @@ -507,6 +524,15 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > vcdt2 |= vcdt_part << ((i % 2) * 16);
> > }
> >
> > + if (priv->mf.field != V4L2_FIELD_NONE) {
> > + fld = FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN;
> > +
> > + if (priv->std & V4L2_STD_525_60)
> > + fld |= FLD_FLD_NUM(2);
> > + else
> > + fld |= FLD_FLD_NUM(1);
> > + }
> > +
> > phycnt = PHYCNT_ENABLECLK;
> > phycnt |= (1 << priv->lanes) - 1;
> >
> > @@ -519,8 +545,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > rcsi2_write(priv, PHTC_REG, 0);
> >
> > /* Configure */
> > - rcsi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> > - FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> > + rcsi2_write(priv, FLD_REG, fld);
> > rcsi2_write(priv, VCDT_REG, vcdt);
> > if (vcdt2)
> > rcsi2_write(priv, VCDT2_REG, vcdt2);
> > @@ -662,6 +687,8 @@ static int rcsi2_get_pad_format(struct v4l2_subdev *sd,
> > }
> >
> > static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = {
> > + .s_std = rcsi2_s_std,
> > + .g_std = rcsi2_g_std,
> > .s_stream = rcsi2_s_stream,
> > };
> >
> >
>
--
Regards,
Niklas Söderlund
Re: [PATCH/RFC 10/15] drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3
Hi Laurent,
On Thu, Mar 07, 2019 at 01:23:40AM +0200, Laurent Pinchart wrote:
> On the D3 SoC the LVDS PHY must be enabled in the same register write
> that enables the LVDS output. Skip writing the LVEN bit independently
> on that platform, it will be set by the write that sets LVRES.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index b1abe737dc05..5ac92ee15be0 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -475,9 +475,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> }
>
> if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> - /* Turn on the LVDS PHY. */
> + /*
> + * Turn on the LVDS PHY. On D3, the LVEN and LVRES bit must be
> + * set at the same time, so don't write the register yet.
> + */
> lvdcr0 |= LVDCR0_LVEN;
> - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> + if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_PWD))
This is quite obscure, and works because D3 is the only SoC that has
(quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) and (!(quirks & RCAR_LVDS_QUIRK_PWD)).
I guess there are not many ways around this.
> + rcar_lvds_write(lvds, LVDCR0, lvdcr0);
I have verified this against the latest v1.50 datasheet, and it
matches what's reported in section 37.3.7 so please add my:
Reviewed-by: Jacopo Mondi
I would like just to add that the same section prescribes a precise
order in which LVDS0 and LVDS1 have to be configured when working with
vertical stripe output. Is that enforced in this series?
Thanks
j
> }
>
> if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
> --
> Regards,
>
> Laurent Pinchart
>
signature.asc
Description: PGP signature
Re: [PATCH/RFC 09/15] drm: rcar-du: lvds: Adjust operating frequency for D3 and E3
Hi Laurent,
thanks, this matches section 37.1.1 of v1.50 datasheet
Reviewed-by: Jacopo Mondi
Thanks
j
On Thu, Mar 07, 2019 at 01:23:39AM +0200, Laurent Pinchart wrote:
> The D3 and E3 SoCs have different pixel clock frequency limits for the
> LVDS encoder than the other SoCs in the Gen3 family. Adjust the mode
> fixup implementation accordingly.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index cd202157264a..b1abe737dc05 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -515,11 +515,16 @@ static bool rcar_lvds_mode_fixup(struct drm_bridge
> *bridge,
>const struct drm_display_mode *mode,
>struct drm_display_mode *adjusted_mode)
> {
> + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> + int min_freq;
> +
> /*
>* The internal LVDS encoder has a restricted clock frequency operating
> - * range (31MHz to 148.5MHz). Clamp the clock accordingly.
> + * range, from 5MHz to 148.5MHz on D3 and E3, and from 31MHz to
> + * 148.5MHz on all other platforms. Clamp the clock accordingly.
>*/
> - adjusted_mode->clock = clamp(adjusted_mode->clock, 31000, 148500);
> + min_freq = lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL ? 5000 : 31000;
> + adjusted_mode->clock = clamp(adjusted_mode->clock, min_freq, 148500);
>
> return true;
> }
> --
> Regards,
>
> Laurent Pinchart
>
signature.asc
Description: PGP signature
Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
Hi Laurent, On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote: > The THC63LVD1024 LVDS decoder can operate in two modes, single-link or > dual-link. In dual-link mode both input ports are used to carry even- > and odd-numbered pixels separately. Document this in the DT bindings, > along with the related rules governing port and usage. > > Signed-off-by: Laurent Pinchart > --- > .../bindings/display/bridge/thine,thc63lvd1024.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > index 37f0c04d5a28..4ff6eb9bbc19 100644 > --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > @@ -28,6 +28,13 @@ Optional video port nodes: > - port@1: Second LVDS input port > - port@3: Second digital CMOS/TTL parallel output > > +The device can operate in single-link mode or dual-link mode. In single-link > +mode, all pixels are received on port@0, and port@1 shall not contain any > +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and > +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain > +endpoints. You know, I'm not sure this is helpful, as if we have to go and describe what the chip supports, a paragraph for dual ouput mode would be required as well. The bindings already document that the chip supports single/dual input/output modes, maybe you can just add rules that prescribes how to populate the endpoints, for both input and output modes? > + > + You can drop this empty line. Thanks j > Example: > > > -- > Regards, > > Laurent Pinchart > signature.asc Description: PGP signature
Re: [PATCH V2] PCI: rcar: Add the initialization of PCIe link in resume_noirq
[+cc linux-pm, Rafael for SET_NOIRQ_SYSTEM_SLEEP_PM_OPS question at the end]
On Thu, Mar 07, 2019 at 11:49:34PM +0100, Marek Vasut wrote:
> On 3/7/19 9:50 PM, Bjorn Helgaas wrote:
> > On Sun, Feb 17, 2019 at 02:24:41PM +0100, marek.va...@gmail.com wrote:
> >> From: Kazufumi Ikeda
> >>
> >> Reestablish the PCIe link very early in the resume process in case it
> >> went down to prevent PCI accesses from hanging the bus. Such accesses
> >> can happen early in the PCI resume process, in the resume_noirq, thus
> >> the link must be reestablished in the resume_noirq callback of the
> >> driver.
> >>
> >> Signed-off-by: Kazufumi Ikeda
> >> Signed-off-by: Gaku Inami
> >> Signed-off-by: Marek Vasut
> >> Cc: Geert Uytterhoeven
> >> Cc: Phil Edworthy
> >> Cc: Simon Horman
> >> Cc: Wolfram Sang
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> ---
> >> V2: - Use BIT() macro for (1 << n)
> >> - Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
> >> add extra changes to this function anymore
> >> - Make resume_noirq return early and clean up parenthesis therein
> >> ---
> >> drivers/pci/controller/pcie-rcar.c | 20
> >> 1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/pcie-rcar.c
> >> b/drivers/pci/controller/pcie-rcar.c
> >> index c8febb009454..b8f8fb3bc640 100644
> >> --- a/drivers/pci/controller/pcie-rcar.c
> >> +++ b/drivers/pci/controller/pcie-rcar.c
> >> @@ -46,6 +46,7 @@
> >>
> >> /* Transfer control */
> >> #define PCIETCTLR 0x02000
> >> +#define DL_DOWN BIT(3)
> >> #define CFINIT 1
> >
> > I saw discussion after the V1 patch about using BIT() and making
> > similar constants also use BIT() for consistency. That makes sense to
> > me, and I think the best way would be:
> >
> > 1) in *this* patch, use "#define DL_DOWN 8"
> > 2) in a followup patch, convert them all to BIT()
> >
> > That way each revision of pcie-rcar.c is self-consistent.
>
> But the BIT() macros are already cleaned , see commit
> 0ee40820989b330e24926d82953ffb9e1c7a8425
>
> PCI: rcar: Clean up the macros
Hmmm. Maybe I'm missing something, but it looks like 0ee40820989b
didn't touch CFINIT, DATA_LINK_ACTIVE, or MSIFE. Arguably,
LINK_SPEED_2_5GTS and LINK_SPEED_5_0GTS could use BIT() also.
I guess I'm just old-school, but in my personal opinion, BIT() is more
trouble than it's worth. I'd rather see a complete bitmask because I
can easily match it with the typical pictures in a spec, multi-bit
fields are easy (you don't have to mix BIT() and GENMASK()), it gives
a hint about the register width, it's easy to match with a hexdump,
etc, e.g.,
#define DL_DOWN0x0008
#define CFINIT 0x0001
But I'm not suggesting that you get rid of BIT() in this driver. I'm
fine with it as long as it's used consistently.
BTW, while we're looking at it, I think rcar_pci_read_reg() and
rcar_pci_write_reg() really should use "u32" instead of "unsigned
long", since they deal with hardware registers that are explicitly
32 bits wide.
> >> #define PCIETSTR 0x02004
> >> #define DATA_LINK_ACTIVE 1
> >> @@ -1130,6 +1131,7 @@ static int rcar_pcie_probe(struct platform_device
> >> *pdev)
> >>pcie = pci_host_bridge_priv(bridge);
> >>
> >>pcie->dev = dev;
> >> + platform_set_drvdata(pdev, pcie);
> >>
> >>err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
> >>if (err)
> >> @@ -1221,10 +1223,28 @@ static int rcar_pcie_probe(struct platform_device
> >> *pdev)
> >>return err;
> >> }
> >>
> >> +static int rcar_pcie_resume_noirq(struct device *dev)
> >> +{
> >> + struct rcar_pcie *pcie = dev_get_drvdata(dev);
> >> +
> >> + if (rcar_pci_read_reg(pcie, PMSR) &&
> >> + !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
> >> + return 0;
> >> +
> >> + /* Re-establish the PCIe link */
> >> + rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
> >> + return rcar_pcie_wait_for_dl(pcie);
> >> +}
> >> +
> >> +static const struct dev_pm_ops rcar_pcie_pm_ops = {
> >> + .resume_noirq = rcar_pcie_resume_noirq,
> >> +};
> >
> > I think there's the beginning of a convention to use #ifdef
> > CONFIG_PM_SLEEP around the ops themselves [1]. Otherwise I think
> > we'll get a warning about unused code when CONFIG_PM_SLEEP is unset.
>
> Only if I used SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() , but I set the
> resume_noirq directly.
Fair enough. I guess in this case if CONFIG_PM_SLEEP is unset, you
set the pointer, which avoids the "unused function" warning, but we
just never use that function pointer.
My intent is to avoid needless differences when possible, so when I
review things like this I look at how other drivers do things. It
looks like all the other controllers use
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() or similar:
git grep -A3 "static.*dev_pm_ops" drivers/pci/controller
In the rcar case you only need the resume_fn, not the suspend_fn, so
SET_NOIRQ_SYSTEM_SLEEP_
Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Laurent,
On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> separate link.
>
> To implement support for this mode of operation, determine if the LVDS
> connection operates in dual-link mode by querying the next device in the
> pipeline, locate the companion encoder, and control it directly through
> its bridge operations.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 104
> drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++
> 2 files changed, 96 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 5ac92ee15be0..90d33514bb3e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -66,6 +66,9 @@ struct rcar_lvds {
>
> struct drm_display_mode display_mode;
> enum rcar_lvds_mode mode;
> +
> + struct drm_bridge *companion;
> + bool dual_link;
> };
>
> #define bridge_to_rcar_lvds(bridge) \
> @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> const struct drm_display_mode *mode = &lvds->display_mode;
> - /*
> - * FIXME: We should really retrieve the CRTC through the state, but how
> - * do we get a state pointer?
> - */
> - struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> u32 lvdhcr;
> u32 lvdcr0;
> int ret;
> @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> if (ret < 0)
> return;
>
> + /* Enable the companion LVDS encoder in dual-link mode. */
> + if (lvds->dual_link && lvds->companion)
> + lvds->companion->funcs->enable(lvds->companion);
> +
> /*
>* Hardcode the channels and control signals routing for now.
>*
> @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>
> if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> - /* Disable dual-link mode. */
> - rcar_lvds_write(lvds, LVDSTRIPE, 0);
> + /*
> + * Configure vertical stripe based on the mode of operation of
> + * the connected device.
> + */
> + rcar_lvds_write(lvds, LVDSTRIPE,
> + lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> }
>
> - /* PLL clock configuration. */
> - lvds->info->pll_setup(lvds, mode->clock * 1000);
> + /*
> + * PLL clock configuration on all instances but the companion in
> + * dual-link mode.
> + */
> + if (!lvds->dual_link || lvds->companion)
> + lvds->info->pll_setup(lvds, mode->clock * 1000);
>
> /* Set the LVDS mode and select the input. */
> lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> - if (drm_crtc_index(crtc) == 2)
> - lvdcr0 |= LVDCR0_DUSEL;
> +
> + if (lvds->bridge.encoder) {
> + /*
> + * FIXME: We should really retrieve the CRTC through the state,
> + * but how do we get a state pointer?
> + */
> + if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> + lvdcr0 |= LVDCR0_DUSEL;
> + }
> +
> rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>
> /* Turn all the channels on. */
> @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> rcar_lvds_write(lvds, LVDCR1, 0);
> rcar_lvds_write(lvds, LVDPLLCR, 0);
>
> + /* Disable the companion LVDS encoder in dual-link mode. */
> + if (lvds->dual_link && lvds->companion)
> + lvds->companion->funcs->disable(lvds->companion);
> +
> clk_disable_unprepare(lvds->clocks.mod);
> }
>
> @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs
> rcar_lvds_bridge_ops = {
> .mode_set = rcar_lvds_mode_set,
> };
>
> +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> +{
> + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +
> + return lvds->dual_link;
> +}
> +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> +
> /*
> -
> * Probe & Remove
> */
>
> +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> +{
> + const struct of_device_id *match;
> + struct device_node *companion;
> + struct device *dev = lvds->dev;
> + int ret = 0;
> +
> + /* Locate the companion LVDS encoder for dual-link operation, if any. */
> + companion = of_parse_phandle(dev->of_node, "renesas,companion", 0);
> + if (!companion)
> + return -ENODEV;
> +
> + /*
> + * Sanity check: the companion encoder must have the same compatible
> + * string.
> +
Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings
Hi Laurent,
On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> input bus mode (single-link or dual-link). The other fields of the
> timings structure are kept to 0 as they do not apply to LVDS buses.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ---
> 1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c
> b/drivers/gpu/drm/bridge/thc63lvd1024.c
> index b083a740565c..206b0af5e154 100644
> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -31,6 +31,8 @@ struct thc63_dev {
>
> struct drm_bridge bridge;
> struct drm_bridge *next;
> +
> + struct drm_bridge_timings timings;
> };
>
> static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> const struct drm_display_mode *mode)
> {
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + unsigned int min_freq;
> + unsigned int max_freq;
> +
> /*
> - * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> - * mode. Note that the limits are different in dual-in, single-out mode,
> - * and will need to be adjusted accordingly.
> + * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> + * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> + * isn't supported by the driver yet, simply derive the limits from the
> + * input mode.
>*/
> - if (mode->clock < 8000)
> + if (thc63->timings.dual_link) {
> + min_freq = 4;
> + max_freq = 15;
> + } else {
> + min_freq = 8000;
> + max_freq = 135000;
> + }
> +
> + if (mode->clock < min_freq)
> return MODE_CLOCK_LOW;
>
> - if (mode->clock > 135000)
> + if (mode->clock > max_freq)
> return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func
> = {
>
> static int thc63_parse_dt(struct thc63_dev *thc63)
> {
> - struct device_node *thc63_out;
> + struct device_node *endpoint;
> struct device_node *remote;
>
> - thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> - THC63_RGB_OUT0, -1);
> - if (!thc63_out) {
> + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> + THC63_RGB_OUT0, -1);
> + if (!endpoint) {
> dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> THC63_RGB_OUT0);
> return -ENODEV;
> }
>
> - remote = of_graph_get_remote_port_parent(thc63_out);
> - of_node_put(thc63_out);
> + remote = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> if (!remote) {
> dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> THC63_RGB_OUT0);
> @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> if (!thc63->next)
> return -EPROBE_DEFER;
>
> + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> + THC63_LVDS_IN1, -1);
> + of_node_put(endpoint);
> +
Should you check if endpoint is enabled?
By the way, this seems to me works properly, as in [12/15] if skip
creation of the LVDS1 encoder if it is operating in dual link mode.
Without that, we would hit again the issue of matching on device
nodes, as the same bridge would be attached twice, am I wrong?
I feel we've been there already :)
https://lkml.org/lkml/2018/3/20/341
and it is not just an issue on matching on endpoints, it's that the
drm bridge itself that would need to handle multiple attach/detaches..
> + thc63->timings.dual_link = endpoint != NULL;
> + dev_dbg(thc63->dev, "operating in %s-link mode\n",
> + thc63->timings.dual_link ? "dual" : "single");
> +
> return 0;
> }
>
> @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> thc63->bridge.driver_private = thc63;
> thc63->bridge.of_node = pdev->dev.of_node;
> thc63->bridge.funcs = &thc63_bridge_func;
> + thc63->bridge.timings = &thc63->timings;
>
> drm_bridge_add(&thc63->bridge);
>
> --
> Regards,
>
> Laurent Pinchart
>
signature.asc
Description: PGP signature
Re: [PATCH v8 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hello!
On 03/08/2019 12:14 PM, Geert Uytterhoeven wrote:
>>> Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
>>>
>>> Signed-off-by: Mason Yang
>>> Signed-off-by: Sergei Shtylyov
>> [...]
>>> diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
>>> new file mode 100644
>>> index 000..ea12017
>>> --- /dev/null
>>> +++ b/drivers/spi/spi-renesas-rpc.c
>>> @@ -0,0 +1,804 @@
>> [...]
>>> +static void rpc_spi_hw_init(struct rpc_spi *rpc)
>>> +{
>>> + //
>>> + // NOTE: The 0x260 are undocumented bits, but they must be set.
>>> + // RPC_PHYCNT_STRTIM is strobe timing adjustment bit,
>>> + // 0x0 : the delay is biggest,
>>> + // 0x1 : the delay is 2nd biggest,
>>> + // On H3 ES1.x, the value should be 0, while on others,
>>> + // the value should be 6.
>>> + //
>>> + regmap_write(rpc->regmap, RPC_PHYCNT, RPC_PHYCNT_CAL |
>>> + RPC_PHYCNT_STRTIM(6) | 0x260);
>>> +
>>> + //
>>> + // NOTE: The 0x1511144 are undocumented bits, but they must be set
>>> + // for RPC_PHYOFFSET1.
>>> + // The 0x31 are undocumented bits, but they must be set
>>> + // for RPC_PHYOFFSET2.
>>> + //
>>> + regmap_write(rpc->regmap, RPC_PHYOFFSET1, RPC_PHYOFFSET1_DDRTMG(3) |
>>> + 0x1511144);
>>> + regmap_write(rpc->regmap, RPC_PHYOFFSET2, 0x31 |
>>> + RPC_PHYOFFSET2_OCTTMG(4));
>>> + regmap_write(rpc->regmap, RPC_SSLDR, RPC_SSLDR_SPNDL(7) |
>>> + RPC_SSLDR_SLNDL(7) | RPC_SSLDR_SCKDL(7));
>>> + regmap_write(rpc->regmap, RPC_CMNCR, RPC_CMNCR_MD | RPC_CMNCR_SFDE |
>>> + RPC_CMNCR_MOIIO_HIZ | RPC_CMNCR_IOFV_HIZ |
>>> + RPC_CMNCR_BSZ(0));
>>> +}
>>
>>We clearly need runtime PM get/put() calls around this code. Otherwise,
>> we're dependant on U-Boot leaving the clocks enabled...
>
> Even that would be futile, as the common clock framework disables all
> unused clocks at late boot time.
This code is executed during the probing time and it indeed works. The
clocks seem
to be disabled somewhat later...
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
Re: [PATCH] ARM: dts: alt: Add DA9063 PMIC node
On 3/8/19 3:53 PM, Wolfram Sang wrote: > >> Do we really want to speed up the PMIC I2C bus ? What are the pros >> (none?) and cons (might be unstable) ? > > Then let's go down to 10kHz instead? Even more stable. 100 kHz and 400 kHz seem to be the most often used I2C frequencies, maybe we should stick to those, since I presume Dialog tested the PMIC with at least those two. -- Best regards, Marek Vasut
Re: [PATCH/RFC 05/15] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
Hi Jacopo, On Fri, Mar 08, 2019 at 05:49:25PM +0100, Jacopo Mondi wrote: > On Thu, Mar 07, 2019 at 01:23:35AM +0200, Laurent Pinchart wrote: > > The THC63LVD1024 LVDS decoder can operate in two modes, single-link or > > dual-link. In dual-link mode both input ports are used to carry even- > > and odd-numbered pixels separately. Document this in the DT bindings, > > along with the related rules governing port and usage. > > > > Signed-off-by: Laurent Pinchart > > --- > > .../bindings/display/bridge/thine,thc63lvd1024.txt | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > index 37f0c04d5a28..4ff6eb9bbc19 100644 > > --- > > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > +++ > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > @@ -28,6 +28,13 @@ Optional video port nodes: > > - port@1: Second LVDS input port > > - port@3: Second digital CMOS/TTL parallel output > > > > +The device can operate in single-link mode or dual-link mode. In > > single-link > > +mode, all pixels are received on port@0, and port@1 shall not contain any > > +endpoint. In dual-link mode, even-numbered pixels are received on port@0 > > and > > +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain > > +endpoints. > > You know, I'm not sure this is helpful, as if we have to go and > describe what the chip supports, a paragraph for dual ouput mode would > be required as well. The bindings already document that the chip > supports single/dual input/output modes, maybe you can just add rules > that prescribes how to populate the endpoints, for both input and > output modes? That's what I was trying to do :-) How else would you like to see it described ? > > + > > + > > You can drop this empty line. > > > Example: > > > > -- Regards, Laurent Pinchart
Re: [PATCH/RFC 06/15] drm: bridge: thc63: Report input bus mode through bridge timings
Hi Jacopo,
On Fri, Mar 08, 2019 at 06:32:59PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:36AM +0200, Laurent Pinchart wrote:
> > Set a drm_bridge_timings in the drm_bridge, and use it to report the
> > input bus mode (single-link or dual-link). The other fields of the
> > timings structure are kept to 0 as they do not apply to LVDS buses.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> > drivers/gpu/drm/bridge/thc63lvd1024.c | 46 ---
> > 1 file changed, 35 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > index b083a740565c..206b0af5e154 100644
> > --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> > @@ -31,6 +31,8 @@ struct thc63_dev {
> >
> > struct drm_bridge bridge;
> > struct drm_bridge *next;
> > +
> > + struct drm_bridge_timings timings;
> > };
> >
> > static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> > @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> > static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> > const struct drm_display_mode *mode)
> > {
> > + struct thc63_dev *thc63 = to_thc63(bridge);
> > + unsigned int min_freq;
> > + unsigned int max_freq;
> > +
> > /*
> > -* The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> > -* mode. Note that the limits are different in dual-in, single-out mode,
> > -* and will need to be adjusted accordingly.
> > +* The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> > +* dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
> > +* isn't supported by the driver yet, simply derive the limits from the
> > +* input mode.
> > */
> > - if (mode->clock < 8000)
> > + if (thc63->timings.dual_link) {
> > + min_freq = 4;
> > + max_freq = 15;
> > + } else {
> > + min_freq = 8000;
> > + max_freq = 135000;
> > + }
> > +
> > + if (mode->clock < min_freq)
> > return MODE_CLOCK_LOW;
> >
> > - if (mode->clock > 135000)
> > + if (mode->clock > max_freq)
> > return MODE_CLOCK_HIGH;
> >
> > return MODE_OK;
> > @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs
> > thc63_bridge_func = {
> >
> > static int thc63_parse_dt(struct thc63_dev *thc63)
> > {
> > - struct device_node *thc63_out;
> > + struct device_node *endpoint;
> > struct device_node *remote;
> >
> > - thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > - THC63_RGB_OUT0, -1);
> > - if (!thc63_out) {
> > + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +THC63_RGB_OUT0, -1);
> > + if (!endpoint) {
> > dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> > THC63_RGB_OUT0);
> > return -ENODEV;
> > }
> >
> > - remote = of_graph_get_remote_port_parent(thc63_out);
> > - of_node_put(thc63_out);
> > + remote = of_graph_get_remote_port_parent(endpoint);
> > + of_node_put(endpoint);
> > if (!remote) {
> > dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> > THC63_RGB_OUT0);
> > @@ -132,6 +147,14 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> > if (!thc63->next)
> > return -EPROBE_DEFER;
> >
> > + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> > +THC63_LVDS_IN1, -1);
> > + of_node_put(endpoint);
> > +
>
> Should you check if endpoint is enabled?
Can endpoints be disabled ?
> By the way, this seems to me works properly, as in [12/15] if skip
> creation of the LVDS1 encoder if it is operating in dual link mode.
> Without that, we would hit again the issue of matching on device
> nodes, as the same bridge would be attached twice, am I wrong?
>
> I feel we've been there already :)
> https://lkml.org/lkml/2018/3/20/341
>
> and it is not just an issue on matching on endpoints, it's that the
> drm bridge itself that would need to handle multiple attach/detaches..
Correct. Thanks to 12/15 we don't need this yet :-)
> > + thc63->timings.dual_link = endpoint != NULL;
> > + dev_dbg(thc63->dev, "operating in %s-link mode\n",
> > + thc63->timings.dual_link ? "dual" : "single");
> > +
> > return 0;
> > }
> >
> > @@ -188,6 +211,7 @@ static int thc63_probe(struct platform_device *pdev)
> > thc63->bridge.driver_private = thc63;
> > thc63->bridge.of_node = pdev->dev.of_node;
> > thc63->bridge.funcs = &thc63_bridge_func;
> > + thc63->bridge.timings = &thc63->timings;
> >
> > drm_bridge_add(&thc63->bridge);
> >
--
Regards,
Laurent Pinchart
Re: [PATCH/RFC 10/15] drm: rcar-du: lvds: Set LVEN and LVRES bits together on D3
Hi Jacopo,
On Fri, Mar 08, 2019 at 05:25:12PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:40AM +0200, Laurent Pinchart wrote:
> > On the D3 SoC the LVDS PHY must be enabled in the same register write
> > that enables the LVDS output. Skip writing the LVEN bit independently
> > on that platform, it will be set by the write that sets LVRES.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index b1abe737dc05..5ac92ee15be0 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -475,9 +475,13 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > }
> >
> > if (lvds->info->quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) {
> > - /* Turn on the LVDS PHY. */
> > + /*
> > +* Turn on the LVDS PHY. On D3, the LVEN and LVRES bit must be
> > +* set at the same time, so don't write the register yet.
> > +*/
> > lvdcr0 |= LVDCR0_LVEN;
> > - rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> > + if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_PWD))
>
> This is quite obscure, and works because D3 is the only SoC that has
> (quirks & RCAR_LVDS_QUIRK_GEN3_LVEN) and (!(quirks & RCAR_LVDS_QUIRK_PWD)).
>
> I guess there are not many ways around this.
We could add a model field to the info structure, or another quirk, but
I'd rather not add yet another if it's not needed for now. I agree it's
not very nice though, it bothered me too when I wrote the code.
> > + rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>
> I have verified this against the latest v1.50 datasheet, and it
> matches what's reported in section 37.3.7 so please add my:
> Reviewed-by: Jacopo Mondi
>
> I would like just to add that the same section prescribes a precise
> order in which LVDS0 and LVDS1 have to be configured when working with
> vertical stripe output. Is that enforced in this series?
It is, the companion is enabled before and disabled after the master for
this reason. My code initially violated the constraint, and the HDMI
output remained blank.
Note that figure 37.9 describes a sequence where register writes are
interleaved. As it's titled "The sample setting of the vertical stripe
output", I've considered it as a sample only.
> > }
> >
> > if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
--
Regards,
Laurent Pinchart
Re: [PATCH/RFC 11/15] drm: rcar-du: lvds: Add support for dual-link mode
Hi Jacopo,
On Fri, Mar 08, 2019 at 06:20:23PM +0100, Jacopo Mondi wrote:
> On Thu, Mar 07, 2019 at 01:23:41AM +0200, Laurent Pinchart wrote:
> > In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> > sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> > separate link.
> >
> > To implement support for this mode of operation, determine if the LVDS
> > connection operates in dual-link mode by querying the next device in the
> > pipeline, locate the companion encoder, and control it directly through
> > its bridge operations.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> > drivers/gpu/drm/rcar-du/rcar_lvds.c | 104
> > drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++
> > 2 files changed, 96 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > index 5ac92ee15be0..90d33514bb3e 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> > @@ -66,6 +66,9 @@ struct rcar_lvds {
> >
> > struct drm_display_mode display_mode;
> > enum rcar_lvds_mode mode;
> > +
> > + struct drm_bridge *companion;
> > + bool dual_link;
> > };
> >
> > #define bridge_to_rcar_lvds(bridge) \
> > @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > {
> > struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > const struct drm_display_mode *mode = &lvds->display_mode;
> > - /*
> > -* FIXME: We should really retrieve the CRTC through the state, but how
> > -* do we get a state pointer?
> > -*/
> > - struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> > u32 lvdhcr;
> > u32 lvdcr0;
> > int ret;
> > @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> > if (ret < 0)
> > return;
> >
> > + /* Enable the companion LVDS encoder in dual-link mode. */
> > + if (lvds->dual_link && lvds->companion)
> > + lvds->companion->funcs->enable(lvds->companion);
> > +
> > /*
> > * Hardcode the channels and control signals routing for now.
> > *
> > @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge
> > *bridge)
> > rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
> >
> > if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> > - /* Disable dual-link mode. */
> > - rcar_lvds_write(lvds, LVDSTRIPE, 0);
> > + /*
> > +* Configure vertical stripe based on the mode of operation of
> > +* the connected device.
> > +*/
> > + rcar_lvds_write(lvds, LVDSTRIPE,
> > + lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> > }
> >
> > - /* PLL clock configuration. */
> > - lvds->info->pll_setup(lvds, mode->clock * 1000);
> > + /*
> > +* PLL clock configuration on all instances but the companion in
> > +* dual-link mode.
> > +*/
> > + if (!lvds->dual_link || lvds->companion)
> > + lvds->info->pll_setup(lvds, mode->clock * 1000);
> >
> > /* Set the LVDS mode and select the input. */
> > lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> > - if (drm_crtc_index(crtc) == 2)
> > - lvdcr0 |= LVDCR0_DUSEL;
> > +
> > + if (lvds->bridge.encoder) {
> > + /*
> > +* FIXME: We should really retrieve the CRTC through the state,
> > +* but how do we get a state pointer?
> > +*/
> > + if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> > + lvdcr0 |= LVDCR0_DUSEL;
> > + }
> > +
> > rcar_lvds_write(lvds, LVDCR0, lvdcr0);
> >
> > /* Turn all the channels on. */
> > @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge
> > *bridge)
> > rcar_lvds_write(lvds, LVDCR1, 0);
> > rcar_lvds_write(lvds, LVDPLLCR, 0);
> >
> > + /* Disable the companion LVDS encoder in dual-link mode. */
> > + if (lvds->dual_link && lvds->companion)
> > + lvds->companion->funcs->disable(lvds->companion);
> > +
> > clk_disable_unprepare(lvds->clocks.mod);
> > }
> >
> > @@ -628,10 +650,54 @@ static const struct drm_bridge_funcs
> > rcar_lvds_bridge_ops = {
> > .mode_set = rcar_lvds_mode_set,
> > };
> >
> > +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> > +{
> > + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> > +
> > + return lvds->dual_link;
> > +}
> > +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> > +
> > /*
> > -
> > * Probe & Remove
> > */
> >
> > +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> > +{
> > + const struct of_device_id *match;
> > + struct device_node *companion;
> > + struct device *dev = lvds->dev;
> > + int ret = 0;
> > +
> > + /* Locate the companion LVDS encoder for dual-link operation, if any. */
> > + companion = of_parse_phandle(dev->of_node, "ren
Re: [PATCH V2] PCI: rcar: Add the initialization of PCIe link in resume_noirq
On 3/8/19 6:17 PM, Bjorn Helgaas wrote:
> [+cc linux-pm, Rafael for SET_NOIRQ_SYSTEM_SLEEP_PM_OPS question at the end]
>
> On Thu, Mar 07, 2019 at 11:49:34PM +0100, Marek Vasut wrote:
>> On 3/7/19 9:50 PM, Bjorn Helgaas wrote:
>>> On Sun, Feb 17, 2019 at 02:24:41PM +0100, marek.va...@gmail.com wrote:
From: Kazufumi Ikeda
Reestablish the PCIe link very early in the resume process in case it
went down to prevent PCI accesses from hanging the bus. Such accesses
can happen early in the PCI resume process, in the resume_noirq, thus
the link must be reestablished in the resume_noirq callback of the
driver.
Signed-off-by: Kazufumi Ikeda
Signed-off-by: Gaku Inami
Signed-off-by: Marek Vasut
Cc: Geert Uytterhoeven
Cc: Phil Edworthy
Cc: Simon Horman
Cc: Wolfram Sang
Cc: linux-renesas-soc@vger.kernel.org
---
V2: - Use BIT() macro for (1 << n)
- Since polling in rcar_pcie_wait_for_dl() uses udelay(), do not
add extra changes to this function anymore
- Make resume_noirq return early and clean up parenthesis therein
---
drivers/pci/controller/pcie-rcar.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/pci/controller/pcie-rcar.c
b/drivers/pci/controller/pcie-rcar.c
index c8febb009454..b8f8fb3bc640 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -46,6 +46,7 @@
/* Transfer control */
#define PCIETCTLR 0x02000
+#define DL_DOWN BIT(3)
#define CFINIT 1
>>>
>>> I saw discussion after the V1 patch about using BIT() and making
>>> similar constants also use BIT() for consistency. That makes sense to
>>> me, and I think the best way would be:
>>>
>>> 1) in *this* patch, use "#define DL_DOWN 8"
>>> 2) in a followup patch, convert them all to BIT()
>>>
>>> That way each revision of pcie-rcar.c is self-consistent.
>>
>> But the BIT() macros are already cleaned , see commit
>> 0ee40820989b330e24926d82953ffb9e1c7a8425
>>
>> PCI: rcar: Clean up the macros
>
> Hmmm. Maybe I'm missing something, but it looks like 0ee40820989b
> didn't touch CFINIT, DATA_LINK_ACTIVE, or MSIFE. Arguably,
> LINK_SPEED_2_5GTS and LINK_SPEED_5_0GTS could use BIT() also.
>
> I guess I'm just old-school, but in my personal opinion, BIT() is more
> trouble than it's worth. I'd rather see a complete bitmask because I
> can easily match it with the typical pictures in a spec, multi-bit
> fields are easy (you don't have to mix BIT() and GENMASK()), it gives
> a hint about the register width, it's easy to match with a hexdump,
> etc, e.g.,
>
> #define DL_DOWN0x0008
> #define CFINIT 0x0001
>
> But I'm not suggesting that you get rid of BIT() in this driver. I'm
> fine with it as long as it's used consistently.
>
> BTW, while we're looking at it, I think rcar_pci_read_reg() and
> rcar_pci_write_reg() really should use "u32" instead of "unsigned
> long", since they deal with hardware registers that are explicitly
> 32 bits wide.
OK, I can send those as separate patches.
#define PCIETSTR 0x02004
#define DATA_LINK_ACTIVE 1
@@ -1130,6 +1131,7 @@ static int rcar_pcie_probe(struct platform_device
*pdev)
pcie = pci_host_bridge_priv(bridge);
pcie->dev = dev;
+ platform_set_drvdata(pdev, pcie);
err = pci_parse_request_of_pci_ranges(dev, &pcie->resources, NULL);
if (err)
@@ -1221,10 +1223,28 @@ static int rcar_pcie_probe(struct platform_device
*pdev)
return err;
}
+static int rcar_pcie_resume_noirq(struct device *dev)
+{
+ struct rcar_pcie *pcie = dev_get_drvdata(dev);
+
+ if (rcar_pci_read_reg(pcie, PMSR) &&
+ !(rcar_pci_read_reg(pcie, PCIETCTLR) & DL_DOWN))
+ return 0;
+
+ /* Re-establish the PCIe link */
+ rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
+ return rcar_pcie_wait_for_dl(pcie);
+}
+
+static const struct dev_pm_ops rcar_pcie_pm_ops = {
+ .resume_noirq = rcar_pcie_resume_noirq,
+};
>>>
>>> I think there's the beginning of a convention to use #ifdef
>>> CONFIG_PM_SLEEP around the ops themselves [1]. Otherwise I think
>>> we'll get a warning about unused code when CONFIG_PM_SLEEP is unset.
>>
>> Only if I used SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() , but I set the
>> resume_noirq directly.
>
> Fair enough. I guess in this case if CONFIG_PM_SLEEP is unset, you
> set the pointer, which avoids the "unused function" warning, but we
> just never use that function pointer.
>
> My intent is to avoid needless differences when possible, so when I
> review things like this I look at how other drivers do things. It
> looks like all the other controllers use
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() or similar:
Re: [PATCH 1/2] [RFC] ata: ahci: Respect bus DMA constraints
On 3/8/19 8:18 AM, Christoph Hellwig wrote: > On Thu, Mar 07, 2019 at 12:14:06PM +0100, Marek Vasut wrote: >>> Right, but whoever *interprets* the device masks after the driver has >>> overridden them should be taking the (smaller) bus mask into account as >>> well, so the question is where is *that* not being done correctly? >> >> Do you have a hint where I should look for that ? > > If this a 32-bit ARM platform it might the complete lack of support > for bus_dma_mask in arch/arm/mm/dma-mapping.c.. It's an ARM 64bit platform, just the PCIe controller is limited to 32bit address range, so the devices on the PCIe bus cannot read the host's DRAM above the 32bit limit. -- Best regards, Marek Vasut
[PATCH v2 2/2] rcar-csi2: Use standby mode instead of resetting
Later versions of the datasheet updates the reset procedure to more
closely resemble the standby mode. Update the driver to enter and exit
the standby mode instead of resetting the hardware before and after
streaming is started and stopped.
While at it break out the full start and stop procedures from
rcsi2_s_stream() into the existing helper functions.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/Kconfig | 1 +
drivers/media/platform/rcar-vin/rcar-csi2.c | 69 +
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/Kconfig
b/drivers/media/platform/rcar-vin/Kconfig
index e3eb8fee253658da..f26f47e3bcf44825 100644
--- a/drivers/media/platform/rcar-vin/Kconfig
+++ b/drivers/media/platform/rcar-vin/Kconfig
@@ -3,6 +3,7 @@ config VIDEO_RCAR_CSI2
tristate "R-Car MIPI CSI-2 Receiver"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
depends on ARCH_RENESAS || COMPILE_TEST
+ select RESET_CONTROLLER
select V4L2_FWNODE
help
Support for Renesas R-Car MIPI CSI-2 receiver.
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index f64528d2be3c95dd..7a1c9b549e0fffc6 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
@@ -350,6 +351,7 @@ struct rcar_csi2 {
struct device *dev;
void __iomem *base;
const struct rcar_csi2_info *info;
+ struct reset_control *rstc;
struct v4l2_subdev subdev;
struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
@@ -387,11 +389,19 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned
int reg, u32 data)
iowrite32(data, priv->base + reg);
}
-static void rcsi2_reset(struct rcar_csi2 *priv)
+static void rcsi2_enter_standby(struct rcar_csi2 *priv)
{
- rcsi2_write(priv, SRST_REG, SRST_SRST);
+ rcsi2_write(priv, PHYCNT_REG, 0);
+ rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
+ reset_control_assert(priv->rstc);
usleep_range(100, 150);
- rcsi2_write(priv, SRST_REG, 0);
+ pm_runtime_put(priv->dev);
+}
+
+static void rcsi2_exit_standby(struct rcar_csi2 *priv)
+{
+ pm_runtime_get_sync(priv->dev);
+ reset_control_deassert(priv->rstc);
}
static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
@@ -462,7 +472,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned
int bpp)
return mbps;
}
-static int rcsi2_start(struct rcar_csi2 *priv)
+static int rcsi2_start_receiver(struct rcar_csi2 *priv)
{
const struct rcar_csi2_format *format;
u32 phycnt, vcdt = 0, vcdt2 = 0;
@@ -506,7 +516,6 @@ static int rcsi2_start(struct rcar_csi2 *priv)
/* Init */
rcsi2_write(priv, TREF_REG, TREF_TREF);
- rcsi2_reset(priv);
rcsi2_write(priv, PHTC_REG, 0);
/* Configure */
@@ -564,19 +573,36 @@ static int rcsi2_start(struct rcar_csi2 *priv)
return 0;
}
+static int rcsi2_start(struct rcar_csi2 *priv)
+{
+ int ret;
+
+ rcsi2_exit_standby(priv);
+
+ ret = rcsi2_start_receiver(priv);
+ if (ret) {
+ rcsi2_enter_standby(priv);
+ return ret;
+ }
+
+ ret = v4l2_subdev_call(priv->remote, video, s_stream, 1);
+ if (ret) {
+ rcsi2_enter_standby(priv);
+ return ret;
+ }
+
+ return 0;
+}
+
static void rcsi2_stop(struct rcar_csi2 *priv)
{
- rcsi2_write(priv, PHYCNT_REG, 0);
-
- rcsi2_reset(priv);
-
- rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
+ v4l2_subdev_call(priv->remote, video, s_stream, 0);
+ rcsi2_enter_standby(priv);
}
static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rcar_csi2 *priv = sd_to_csi2(sd);
- struct v4l2_subdev *nextsd;
int ret = 0;
mutex_lock(&priv->lock);
@@ -586,27 +612,12 @@ static int rcsi2_s_stream(struct v4l2_subdev *sd, int
enable)
goto out;
}
- nextsd = priv->remote;
-
if (enable && priv->stream_count == 0) {
- pm_runtime_get_sync(priv->dev);
-
ret = rcsi2_start(priv);
- if (ret) {
- pm_runtime_put(priv->dev);
+ if (ret)
goto out;
- }
-
- ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
- if (ret) {
- rcsi2_stop(priv);
- pm_runtime_put(priv->dev);
- goto out;
- }
} else if (!enable && priv->stream_count == 1) {
rcsi2_stop(priv);
- v4l2_subdev_call(nextsd, video, s_stream, 0);
- pm_runtime_put(priv->dev);
}
priv->stream_count += enable ? 1 : -1;
@@ -936,
[PATCH v2 0/2] rcar-csi2: Use standby mode instead of resetting
Hi, This small series updates rcar-csi2 to use the standby mode described in later versions of the datasheet. * Changes since v1 - Break up enter and exit of standby mode in two separate functions. - Add "select RESET_CONTROLLER" to Kconfig. Niklas Söderlund (2): dt-bindings: rcar-csi2: List resets as a mandatory property rcar-csi2: Use standby mode instead of resetting .../bindings/media/renesas,rcar-csi2.txt | 3 +- drivers/media/platform/rcar-vin/Kconfig | 1 + drivers/media/platform/rcar-vin/rcar-csi2.c | 69 +++ 3 files changed, 45 insertions(+), 28 deletions(-) -- 2.21.0
[PATCH v2 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property
The resets property will become mandatory to operate the device, list it as such. All device tree source files have always included the reset property so making it mandatory will not introduce any regressions. While at it improve the description for the clocks property. Signed-off-by: Niklas Söderlund --- Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt index d63275e17afdd180..9a0d0531c67df48c 100644 --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt @@ -18,7 +18,8 @@ Mandatory properties - reg: the register base and size for the device registers - interrupts: the interrupt for the device - - clocks: reference to the parent clock + - clocks: A phandle + clock specifier for the module clock + - resets: A phandle + reset specifier for the module reset The device node shall contain two 'port' child nodes according to the bindings defined in Documentation/devicetree/bindings/media/ -- 2.21.0
[PATCH v2] rcar-csi2: Propagate the FLD signal for NTSC and PAL
Depending on which video standard is used the driver needs to setup the
hardware to correctly handle fields. If stream is identified as NTSC
or PAL setup field detection and propagate the field detection signal.
Later versions of the datasheet have been updated to make it clear
that FLD register should be set to 0 when dealing with non-interlaced
field formats.
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-csi2.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
---
Hi,
This patch depends on [PATCH v2 0/2] rcar-csi2: Use standby mode instead of
resetting
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 7a1c9b549e0fffc6..d9b29dbbcc2949de 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -475,7 +475,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned
int bpp)
static int rcsi2_start_receiver(struct rcar_csi2 *priv)
{
const struct rcar_csi2_format *format;
- u32 phycnt, vcdt = 0, vcdt2 = 0;
+ u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
unsigned int i;
int mbps, ret;
@@ -507,6 +507,16 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
vcdt2 |= vcdt_part << ((i % 2) * 16);
}
+ if (priv->mf.field != V4L2_FIELD_NONE &&
+ (priv->mf.height == 240 || priv->mf.height == 288)) {
+ fld = FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN;
+
+ if (priv->mf.height == 240)
+ fld |= FLD_FLD_NUM(2);
+ else
+ fld |= FLD_FLD_NUM(1);
+ }
+
phycnt = PHYCNT_ENABLECLK;
phycnt |= (1 << priv->lanes) - 1;
@@ -519,8 +529,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
rcsi2_write(priv, PHTC_REG, 0);
/* Configure */
- rcsi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
- FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
+ rcsi2_write(priv, FLD_REG, fld);
rcsi2_write(priv, VCDT_REG, vcdt);
if (vcdt2)
rcsi2_write(priv, VCDT2_REG, vcdt2);
--
2.21.0
[PATCH v2 0/3] rcar-csi2: Update start procedures to latest revision of datasheet
Hi, This series update the driver to match changes in the latest datasheet (rev 1.50). All changes are related to register setup when starting the stream. This series depends on [PATCH v2] rcar-csi2: Propagate the FLD signal for NTSC and PAL. Niklas Söderlund (3): rcar-csi2: Update V3M and E3 start procedure rcar-csi2: Update start procedure for H3 ES2 rcar-csi2: Move setting of Field Detection Control Register drivers/media/platform/rcar-vin/rcar-csi2.c | 51 + 1 file changed, 41 insertions(+), 10 deletions(-) -- 2.21.0
[PATCH v2 1/3] rcar-csi2: Update V3M and E3 start procedure
The latest datasheet (rev 1.50) updates the start procedure for V3M and
E3. Update the driver to match these changes.
Signed-off-by: Niklas Söderlund
Reviewed-by: Ulrich Hecht
---
drivers/media/platform/rcar-vin/rcar-csi2.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index d9b29dbbcc2949de..6be81d4839f35a0e 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -922,11 +922,11 @@ static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv,
unsigned int mbps)
static int rcsi2_confirm_start_v3m_e3(struct rcar_csi2 *priv)
{
static const struct phtw_value step1[] = {
- { .data = 0xed, .code = 0x34 },
- { .data = 0xed, .code = 0x44 },
- { .data = 0xed, .code = 0x54 },
- { .data = 0xed, .code = 0x84 },
- { .data = 0xed, .code = 0x94 },
+ { .data = 0xee, .code = 0x34 },
+ { .data = 0xee, .code = 0x44 },
+ { .data = 0xee, .code = 0x54 },
+ { .data = 0xee, .code = 0x84 },
+ { .data = 0xee, .code = 0x94 },
{ /* sentinel */ },
};
--
2.21.0
[PATCH v2 3/3] rcar-csi2: Move setting of Field Detection Control Register
Latest datasheet (rev 1.50) clarifies that the FLD register should be set after LINKCNT. Signed-off-by: Niklas Söderlund Reviewed-by: Ulrich Hecht Reviewed-by: Kieran Bingham --- drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c index 07d5c8c66b7cd382..077e0d344b395b54 100644 --- a/drivers/media/platform/rcar-vin/rcar-csi2.c +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c @@ -529,7 +529,6 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv) rcsi2_write(priv, PHTC_REG, 0); /* Configure */ - rcsi2_write(priv, FLD_REG, fld); rcsi2_write(priv, VCDT_REG, vcdt); if (vcdt2) rcsi2_write(priv, VCDT2_REG, vcdt2); @@ -560,6 +559,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv) rcsi2_write(priv, PHYCNT_REG, phycnt); rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN | LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP); + rcsi2_write(priv, FLD_REG, fld); rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ); rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ); -- 2.21.0
[PATCH v2 2/3] rcar-csi2: Update start procedure for H3 ES2
Latest information from hardware engineers reveals that H3 ES2 and ES3
behave differently when working with link speeds bellow 250 Mpbs.
Add a SoC match for H3 ES2.* and use the correct startup sequence.
Signed-off-by: Niklas Söderlund
Reviewed-by: Kieran Bingham
---
drivers/media/platform/rcar-vin/rcar-csi2.c | 39 ++---
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c
b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 6be81d4839f35a0e..07d5c8c66b7cd382 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -914,6 +914,25 @@ static int rcsi2_init_phtw_h3_v3h_m3n(struct rcar_csi2
*priv, unsigned int mbps)
return rcsi2_phtw_write_array(priv, step2);
}
+static int rcsi2_init_phtw_h3es2(struct rcar_csi2 *priv, unsigned int mbps)
+{
+ static const struct phtw_value step1[] = {
+ { .data = 0xcc, .code = 0xe2 },
+ { .data = 0x01, .code = 0xe3 },
+ { .data = 0x11, .code = 0xe4 },
+ { .data = 0x01, .code = 0xe5 },
+ { .data = 0x10, .code = 0x04 },
+ { .data = 0x38, .code = 0x08 },
+ { .data = 0x01, .code = 0x00 },
+ { .data = 0x4b, .code = 0xac },
+ { .data = 0x03, .code = 0x00 },
+ { .data = 0x80, .code = 0x07 },
+ { /* sentinel */ },
+ };
+
+ return rcsi2_phtw_write_array(priv, step1);
+}
+
static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
{
return rcsi2_phtw_write_mbps(priv, mbps, phtw_mbps_v3m_e3, 0x44);
@@ -976,6 +995,14 @@ static const struct rcar_csi2_info
rcar_csi2_info_r8a7795es1 = {
.num_channels = 4,
};
+static const struct rcar_csi2_info rcar_csi2_info_r8a7795es2 = {
+ .init_phtw = rcsi2_init_phtw_h3es2,
+ .hsfreqrange = hsfreqrange_h3_v3h_m3n,
+ .csi0clkfreqrange = 0x20,
+ .num_channels = 4,
+ .clear_ulps = true,
+};
+
static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
.hsfreqrange = hsfreqrange_m3w_h3es1,
.num_channels = 4,
@@ -1041,11 +1068,15 @@ static const struct of_device_id rcar_csi2_of_table[] =
{
};
MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
-static const struct soc_device_attribute r8a7795es1[] = {
+static const struct soc_device_attribute r8a7795[] = {
{
.soc_id = "r8a7795", .revision = "ES1.*",
.data = &rcar_csi2_info_r8a7795es1,
},
+ {
+ .soc_id = "r8a7795", .revision = "ES2.*",
+ .data = &rcar_csi2_info_r8a7795es2,
+ },
{ /* sentinel */ },
};
@@ -1063,10 +1094,10 @@ static int rcsi2_probe(struct platform_device *pdev)
priv->info = of_device_get_match_data(&pdev->dev);
/*
-* r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
-* have it's own compatible string.
+* The different ES versions of r8a7795 (H3) behave differently but
+* share the same compatible string.
*/
- attr = soc_device_match(r8a7795es1);
+ attr = soc_device_match(r8a7795);
if (attr)
priv->info = attr->data;
--
2.21.0
[PATCH 2/2] PCI: rcar: Replace unsigned long with u32 in register accessors
From: Marek Vasut
Replace unsigned long with u32 in register accessor functions,
since they access 32bit registers.
Signed-off-by: Marek Vasut
Cc: Geert Uytterhoeven
Cc: Phil Edworthy
Cc: Simon Horman
Cc: Wolfram Sang
Cc: linux-renesas-soc@vger.kernel.org
To: linux-...@vger.kernel.org
---
drivers/pci/controller/pcie-rcar.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pcie-rcar.c
b/drivers/pci/controller/pcie-rcar.c
index 5b8736f0cd6b..1408c8aa758b 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -152,14 +152,12 @@ struct rcar_pcie {
struct rcar_msi msi;
};
-static void rcar_pci_write_reg(struct rcar_pcie *pcie, unsigned long val,
- unsigned long reg)
+static void rcar_pci_write_reg(struct rcar_pcie *pcie, u32 val, u32 reg)
{
writel(val, pcie->base + reg);
}
-static unsigned long rcar_pci_read_reg(struct rcar_pcie *pcie,
- unsigned long reg)
+static u32 rcar_pci_read_reg(struct rcar_pcie *pcie, u32 reg)
{
return readl(pcie->base + reg);
}
--
2.20.1
[PATCH 1/2] PCI: rcar: Clean up remaining macros defining bits
From: Marek Vasut Replace macros using constants with BIT()s instead, no functional change. Signed-off-by: Marek Vasut Cc: Geert Uytterhoeven Cc: Phil Edworthy Cc: Simon Horman Cc: Wolfram Sang Cc: linux-renesas-soc@vger.kernel.org To: linux-...@vger.kernel.org --- drivers/pci/controller/pcie-rcar.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c index c8febb009454..5b8736f0cd6b 100644 --- a/drivers/pci/controller/pcie-rcar.c +++ b/drivers/pci/controller/pcie-rcar.c @@ -46,14 +46,14 @@ /* Transfer control */ #define PCIETCTLR 0x02000 -#define CFINIT1 +#define CFINITBIT(0) #define PCIETSTR 0x02004 -#define DATA_LINK_ACTIVE 1 +#define DATA_LINK_ACTIVE BIT(0) #define PCIEERRFR 0x02020 #define UNSUPPORTED_REQUEST BIT(4) #define PCIEMSIFR 0x02044 #define PCIEMSIALR 0x02048 -#define MSIFE 1 +#define MSIFE BIT(0) #define PCIEMSIAUR 0x0204c #define PCIEMSIIER 0x02050 -- 2.20.1
[PATCH] arm64: dts: renesas: r8a77995: draak: Enable CAN0, CAN1
From: Marek Vasut
Enable both CAN0 and CAN1 controllers on R8A77995 Draak board,
since they are available on connectors CN43, CN44 respectively.
Signed-off-by: Marek Vasut
Cc: Geert Uytterhoeven
Cc: Simon Horman
Cc: Wolfram Sang
Cc: linux-renesas-soc@vger.kernel.org
To: linux-arm-ker...@lists.infradead.org
---
.../arm64/boot/dts/renesas/r8a77995-draak.dts | 22 +++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index db2bed1751b8..9a000a75fe8e 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -179,6 +179,18 @@
};
};
+&can0 {
+ pinctrl-0 = <&can0_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+};
+
+&can1 {
+ pinctrl-0 = <&can1_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+};
+
&du {
pinctrl-0 = <&du_pins>;
pinctrl-names = "default";
@@ -375,6 +387,16 @@
};
};
+ can0_pins: can0 {
+ groups = "can0_data_a";
+ function = "can0";
+ };
+
+ can1_pins: can1 {
+ groups = "can1_data_a";
+ function = "can1";
+ };
+
du_pins: du {
groups = "du_rgb888", "du_sync", "du_disp", "du_clk_out_0";
function = "du";
--
2.20.1
[PATCH] media: rcar-vin: fix a potential NULL pointer dereference
In case of_match_node cannot find a match, the fix returns -EINVAL to avoid NULL pointer dereference. Signed-off-by: Kangjie Lu --- drivers/media/platform/rcar-vin/rcar-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c index f0719ce24b97..a058e2023ca8 100644 --- a/drivers/media/platform/rcar-vin/rcar-core.c +++ b/drivers/media/platform/rcar-vin/rcar-core.c @@ -266,6 +266,8 @@ static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin) match = of_match_node(vin->dev->driver->of_match_table, vin->dev->of_node); + if (unlikely(!match)) + return -EINVAL; strscpy(mdev->driver_name, KBUILD_MODNAME, sizeof(mdev->driver_name)); strscpy(mdev->model, match->compatible, sizeof(mdev->model)); -- 2.17.1
[PATCH] media: renesas-ceu: fix a potential NULL pointer dereference
In case of_match_device cannot find a match, the check returns
-EINVAL to avoid a potential NULL pointer dereference
Signed-off-by: Kangjie Lu
---
drivers/media/platform/renesas-ceu.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/renesas-ceu.c
b/drivers/media/platform/renesas-ceu.c
index 150196f7cf96..4aa807c0b6c7 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -1682,7 +1682,10 @@ static int ceu_probe(struct platform_device *pdev)
if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
ceu_data = of_match_device(ceu_of_match, dev)->data;
- num_subdevs = ceu_parse_dt(ceudev);
+ if (unlikely(!ceu_data))
+ num_subdevs = -EINVAL;
+ else
+ num_subdevs = ceu_parse_dt(ceudev);
} else if (dev->platform_data) {
/* Assume SH4 if booting with platform data. */
ceu_data = &ceu_data_sh4;
--
2.17.1
