Hi Hans,
On Sat, Mar 27, 2021 at 11:05:01AM +0100, Hans Verkuil wrote:
> On 21/10/2020 23:43, Laurent Pinchart wrote:
> > On Wed, Oct 21, 2020 at 02:53:27PM +0100, Fabrizio Castro wrote:
> >> Dear All,
> >>
> >> this series is to add DRIF support for the r8a779
y.
>
> Signed-off-by: Jacopo Mondi
Tested by applying and verifying that `git show -b` shows an empty diff.
Reviewed-by: Laurent Pinchart
> ---
> .../bindings/media/i2c/maxim,max9286.yaml | 214 +-
> 1 file changed, 107 insertions(+), 107 deletions(-
the hardware level, so including it here sounds good.
The DT binding even makes the port mandatory :-)
Reviewed-by: Laurent Pinchart
> ---
> arch/arm64/boot/dts/renesas/r8a77970.dtsi | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a7797
On Thu, Apr 15, 2021 at 06:47:48PM +0200, Geert Uytterhoeven wrote:
> On Thu, Apr 15, 2021 at 4:47 PM Laurent Pinchart wrote:
> > On Thu, Apr 15, 2021 at 02:25:59PM +0200, Jacopo Mondi wrote:
> > > Declare port@0 in the csi40 device node and leave it un-connected.
> > >
Hi Jacopo,
On Thu, Apr 15, 2021 at 08:58:48AM +0200, Jacopo Mondi wrote:
> On Thu, Apr 15, 2021 at 03:00:56AM +0300, Laurent Pinchart wrote:
> > On Wed, Apr 14, 2021 at 03:51:25PM +0200, Jacopo Mondi wrote:
> > > The 'maxim,gpio-poc' property is used when the remote
*/
> max9286_configure_i2c(priv, false);
>
> - ret = max9286_register_gpio(priv);
> + ret = max9286_parse_gpios(priv);
> if (ret)
> goto err_powerdown;
>
> - priv->regulator = devm_regulator_get(&client->dev, "poc");
> - if (IS_ERR(priv->regulator)) {
> - if (PTR_ERR(priv->regulator) != -EPROBE_DEFER)
> - dev_err(&client->dev,
> - "Unable to get PoC regulator (%ld)\n",
> - PTR_ERR(priv->regulator));
> - ret = PTR_ERR(priv->regulator);
> - goto err_powerdown;
> - }
> -
> ret = max9286_parse_dt(priv);
> if (ret)
> goto err_powerdown;
> @@ -1326,7 +1393,7 @@ static int max9286_remove(struct i2c_client *client)
>
> max9286_v4l2_unregister(priv);
>
> - regulator_disable(priv->regulator);
> + max9286_poc_enable(priv, false);
>
> gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
>
--
Regards,
Laurent Pinchart
rs/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 1 +
> include/drm/bridge/dw_hdmi.h | 2 ++
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
--
Regards,
Laurent Pinchart
isp_layer_id instead
>
> Cc: Hyun Kwon
> Cc: Laurent Pinchart
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Michal Simek
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Signed-off-by: Lee Jones
Reviewed-by: Laurent Pinchart
> ---
in() instead
>
> Cc: Hyun Kwon
> Cc: Laurent Pinchart
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Michal Simek
> Cc: Philipp Zabel
> Cc: dri-de...@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Signed-off-by: Lee Jones
Reviewed-by: Laurent
Hi Jacopo,
On Fri, Apr 16, 2021 at 09:43:07AM +0200, Jacopo Mondi wrote:
> On Thu, Apr 15, 2021 at 10:14:05PM +0300, Laurent Pinchart wrote:
> > > > > + /* GPIO values default to high */
> > > > > + priv->gpio_state = BIT(0) |
TUS(5));
> + iss_print_register(iss, HL_IRQENABLE_SET(5));
> + iss_print_register(iss, HL_IRQENABLE_CLR(5));
> + iss_print_register(iss, CTRL);
> + iss_print_register(iss, CLKCTRL);
> + iss_print_register(iss, CLKSTAT);
>
> dev_dbg(iss->dev, "---\n");
> }
--
Regards,
Laurent Pinchart
Hi Aline,
On Mon, Apr 12, 2021 at 10:58:45AM -0300, ascordeiro wrote:
> Em seg, 2021-04-12 às 16:40 +0300, Laurent Pinchart escreveu:
> > While testing on a device isn't a requirement as you can't be expected
> > to have the necessary hardware, changes are expected t
width than "VIDEO"
> mode.
> +TNR will be enabled in "VIDEO" mode and bypassed by "STILL" mode. ImgU is
> +running at “VIDEO” mode by default, the user can use v4l2 control
> +V4L2_CID_INTEL_IPU3_MODE (currently defined in
> +drivers/staging/media/ipu3
ault";
> +pinctrl-0 = <&ite_pins_default>;
> +vcn33-supply = <&mt6358_vcn33_wifi_reg>;
> +vcn18-supply = <&mt6358_vcn18_reg>;
> +vrf12-supply = <&mt6358_vrf12_reg>;
> +reset-gpios = <&pio 1
gt; > +vcn33-supply = <&mt6358_vcn33_wifi_reg>;
> > +vcn18-supply = <&mt6358_vcn18_reg>;
> > + vrf12-supply = <&mt6358_vrf12_reg>;
> > +reset-gpios = <&pio 160 1 /* GPIO_ACTIVE_LOW */>;
> > +interrupt-parent = <&pio>;
> > +interrupts = <4 8 /* IRQ_TYPE_LEVEL_LOW */>;
> > +reg = <0x4c>;
> > +
> > +ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > +reg = <0>;
> > +it66121_in: endpoint {
> > + bus-width = <12>;
> > + remote-endpoint = <&display_out>;
> > +};
> > + };
> > +
> > + port@1 {
> > +reg = <1>;
> > +hdmi_conn_out: endpoint {
> > + remote-endpoint = <&hdmi_conn_in>;
> > +};
> > + };
> > +};
> > + };
> > +};
--
Regards,
Laurent Pinchart
about hot plug detection
> >>> events.
> >>
> >> It's implemented in the IRQ handler with the
> >> IT66121_INT_STATUS1_HPD_STATUS event.
> >
> > I didn't even get that far :)
> >
> > Either way, the HPD support should be exposed in drm_bridge_funcs
> > (.hpd_enable, .hpd_disable (and possibly .hpd_notify)) and
> > drm_bridge.ops (DRM_BRIDGE_OP_HPD).
>
> Indeed I forgot these calls in the NO_CONNECTOR implementation...
For new bridges, you should no implement connector creation, only the
DRM_BRIDGE_ATTACH_NO_CONNECTOR case should be supported.
--
Regards,
Laurent Pinchart
subject line,
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/rdacm21.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/rdacm21.c b/drivers/media/i2c/rdacm21.c
> index 553e3f03752b..6be8ce130e78 100644
> --- a/drivers/me
853b3b4b ("media: i2c: Add driver for RDACM21 camera module")
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/rdacm21.c | 46 ++---
> 1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --
tion on the control channel
> more reliable.
>
> Reviewed-by: Kieran Bingham
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/rdacm20.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/dri
#address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@4 {
> + reg = <4>;
> + };
> +};
> +
> +i2c-mux {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +};
> + };
> };
It's customary to indent DT examples with 4 spaces. The existing
examples use two spaces, so maybe a patch on top of this would be useful
to increase readability ?
Reviewed-by: Laurent Pinchart
--
Regards,
Laurent Pinchart
d
> after these definitions.
>
> Signed-off-by: Kieran Bingham
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> .../arm64/boot/dts/renesas/r8a77970-eagle.dts | 119 ++
> 1 file changed, 119 insertions(+)
>
> diff --git a/arch/arm64/boo
gt;
> status = "okay";
> };
> +
> +/* FAKRA Overlay */
> +#include "eagle-gmsl.dtsi"
--
Regards,
Laurent Pinchart
remote-endpoint =
> <&max9286_in2>;
> + };
> + };
> + };
> + };
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_3
> + i2c@3 {
> + status = "okay";
> +
> + camera@54 {
> + compatible = EAGLE_CAMERA_MODEL;
> + reg = <0x54>, <0x64>;
> +
> + port {
> + fakra_con3: endpoint {
> + remote-endpoint =
> <&max9286_in3>;
> + };
> + };
> + };
> + };
> +#endif
> + };
> +};
> +#endif
--
Regards,
Laurent Pinchart
uct i2c_client *client)
>*/
> max9286_configure_i2c(priv, false);
>
> - ret = max9286_register_gpio(priv);
> + ret = max9286_parse_gpios(priv);
> if (ret)
> goto err_powerdown;
>
> - priv->regulator = devm_regulator_get(&client->dev, "poc");
> - if (IS_ERR(priv->regulator)) {
> - if (PTR_ERR(priv->regulator) != -EPROBE_DEFER)
> - dev_err(&client->dev,
> - "Unable to get PoC regulator (%ld)\n",
> - PTR_ERR(priv->regulator));
> - ret = PTR_ERR(priv->regulator);
> - goto err_powerdown;
> - }
> -
> ret = max9286_parse_dt(priv);
> if (ret)
> goto err_powerdown;
> @@ -1326,7 +1393,7 @@ static int max9286_remove(struct i2c_client *client)
>
> max9286_v4l2_unregister(priv);
>
> - regulator_disable(priv->regulator);
> + max9286_poc_enable(priv, false);
>
> gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
>
--
Regards,
Laurent Pinchart
ice connected to the DDC/AUX port to read the
EDID and provide modes. The panel driver won't be able to handle it on
its own.
> >> - ddc driver on i2c request should power up the panel - seems also correct,
> >
> > Right, so I could put the
> > "drm_bridge_chain_pre_enable(&pdata->bridge)" into the
> > ti_sn_aux_transfer() function. I talked about that a little bit "after
> > the cut" in my post where I said:
> >
> >> - If everyone loves the "runtime PM" in the panel driver then we
> >> could, in theory, put the pre-enable chaining straight in the "aux
> >> transfer" function.
> >
> > The reason for the dependence on "runtime PM" in the panel driver is
> > that we are doing DDC over AUX and it breaks the EDID reading into
> > lots of chunks so if we did the powering up and powering down there it
> > would be crazy slow without the delayed poweroff.
>
> OK, it resembles to me DSI-controlled panel - to query/configure panel
> panel driver asks DSI-host to transfer some bytes to the panel and/or
> back via DSI-bus.
>
> In case of eDP panels we could do similar thing to read edid - we call
> drm_panel_get_modes - it calls drm_panel_funcs.get_modes callback and it
> decides (based on DT) if it should fill modes according to hardcoded
> info into the driver or to ask the physical panel via DP-controller -
> this way all the players (the panel, AUX/DDC device) will know what to
> power-up.
>
> I guess there is missing pieces - there is no DP bus :), I am not sure
> if there is straight way to access panel's aux/ddc from the panel
> driver, maybe somehow via drm_connector ???
If the SN65DSI86 has to call drm_panel_get_modes(), which will then call
back into the SN65DSI86 driver to perform the EDID read, it seems to me
that the panel driver shouldn't be involved at all.
DRM bridges have "recently" gained new operations to retrieve EDID, and
there's a helper (drm_bridge_connector) that creates a connector for a
chain of bridges, delegating connector operations to the appropriate
bridge in the chain. This seems a better way forward to me (but I'm
biased, as I've authored that code :-)).
> Of course this only my idea - to be discussed with others.
--
Regards,
Laurent Pinchart
t; how to enable (or disable) this new delayed behavior selectively.
> - In order for this to work we now have a hard dependency on
> "PM". From memory this is a legit thing to assume these days and we
> don't have to find some fallback to keep working if someone wants t
Hi Doug,
On Wed, Apr 14, 2021 at 06:22:57PM -0700, Doug Anderson wrote:
> On Wed, Apr 14, 2021 at 5:58 PM Laurent Pinchart wrote:
> > On Fri, Apr 02, 2021 at 03:28:46PM -0700, Douglas Anderson wrote:
> > > Unpreparing and re-preparing a panel can be a really heavy
>
Hi Doug,
On Wed, Apr 14, 2021 at 06:19:13PM -0700, Doug Anderson wrote:
> On Sun, Apr 4, 2021 at 5:50 PM Laurent Pinchart wrote:
> > On Fri, Apr 02, 2021 at 03:28:35PM -0700, Douglas Anderson wrote:
> > > The drm_bridge_chain_pre_enable() is not the
ection status occurs.
>*
>* This callback is optional and shall only be implemented by bridges
> @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
> enum drm_connector_status status),
> void *data);
> void drm_bridge_hpd_disable(struct drm_bridge *bridge);
> -void drm_bridge_hpd_notify(struct drm_bridge *bridge,
> -enum drm_connector_status status);
> +void drm_bridge_hpd_cb(struct drm_bridge *bridge,
> +enum drm_connector_status status);
>
> #ifdef CONFIG_DRM_PANEL_BRIDGE
> struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
--
Regards,
Laurent Pinchart
t? I'll use Connector's ".atomic_check()"
> interface to detect Content Protection property change.
> (https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2674580)
Please note that upstream review happens on mailing lists, not in
gerrit. Internal reviews for Chrome OS development are certainly fine
there, but that will not mean the patch will then be accepted upstream
as-is, it will still need to go through the upstream review process,
without any shortcut. I strongly recommend using an upstream-first
strategy, with public review.
> > > > > +
> > > > > if (ctx->pdata.is_dpi)
> > > > > ret = anx7625_dpi_config(ctx);
> > > > > else
> >
> > /snip
--
Regards,
Laurent Pinchart
M2x")
> Signed-off-by: Mauro Carvalho Chehab
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1644b6e9697c..b405ee71f730 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15258,7 +1
On Fri, Apr 02, 2021 at 12:01:35PM +0300, Laurent Pinchart wrote:
> Hi Mauro,
>
> Thank you for the patch.
>
> On Thu, Apr 01, 2021 at 02:17:44PM +0200, Mauro Carvalho Chehab wrote:
> > The file name: Documentation/devicetree/bindings/media/i2c/rdacm2x-gmsl.yaml
&
be useful to include it in the
example below, to show how it's supposed to be used.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - dmas
> > + - dma-names
> > + - power-domains
> > + - "#address-cells"
> > + - "#size-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +#include
> > +
> > +ti_csi2rx0: ticsi2rx {
> > +compatible = "ti,csi2rx";
> > +dmas = <&main_udmap 0x4940>;
> > +dma-names = "rx0";
> > +reg = <0x0 0x450 0x0 0x1000>;
> > +power-domains = <&k3_pds 26 TI_SCI_PD_EXCLUSIVE>;
> > +#address-cells = <2>;
> > +#size-cells = <2>;
> > +};
--
Regards,
Laurent Pinchart
compatible
> + - reg
> + - clocks
> + - clock-names
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> +dphy0: dphy@fd0e{
This is copied verbatim from the existing description, but while at it,
I'd rename the
- "#phy-cells"
>
> additionalProperties: false
--
Regards,
Laurent Pinchart
cdns,dphy.yaml
> +++ b/Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> @@ -30,6 +30,9 @@ properties:
>"#phy-cells":
> const: 0
>
> + power-domains:
> +maxItems: 1
> +
Would it be useful to add power-domains to the example ?
Reviewed-by: Lau
SUPP;
> + }
> +
> + return 0;
> +}
> +
> static const struct phy_ops cdns_dphy_ops = {
> .configure = cdns_dphy_configure,
> .validate = cdns_dphy_validate,
> .power_on = cdns_dphy_power_on,
> .power_off = cdns_dphy_power_off,
> + .set_mode = cdns_dphy_set_mode,
> };
>
> static int cdns_dphy_probe(struct platform_device *pdev)
--
Regards,
Laurent Pinchart
d_ops = {
> + .enum_mbus_code = csi2rx_enum_mbus_code,
> + .get_fmt= csi2rx_get_fmt,
> + .set_fmt= csi2rx_set_fmt,
> + .enum_frame_size = csi2rx_enum_frame_size,
> + .enum_frame_interval = csi2rx_enum_frame_interval,
> };
>
> static const struct v4l2_subdev_ops csi2rx_subdev_ops = {
> .video = &csi2rx_video_ops,
> + .pad= &csi2rx_pad_ops,
> };
>
> static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
--
Regards,
Laurent Pinchart
On Fri, Apr 02, 2021 at 01:01:22PM +0300, Laurent Pinchart wrote:
> On Thu, Apr 01, 2021 at 10:52:01AM -0500, Rob Herring wrote:
> > On Tue, Mar 30, 2021 at 11:03:44PM +0530, Pratyush Yadav wrote:
> > > TI's J721E uses the Cadence CSI2RX and DPHY peripherals to facilitate
&
ll(csi2rx->source_subdev, core, s_power, false);
> + if (ret && ret != -ENOIOCTLCMD)
> + dev_warn(csi2rx->dev, "Couldn't power off subdev\n");
> +
> if (csi2rx->dphy) {
> writel(0, csi2rx->base + CSI2RX_DPHY_LANE_CTRL_REG);
>
--
Regards,
Laurent Pinchart
; include/linux/phy/phy-mipi-dphy.h | 13 +
> > 11 files changed, 1754 insertions(+), 70 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/ti,csi2rx.yaml
> > delete mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.txt
> > create mode 100644 Documentation/devicetree/bindings/phy/cdns,dphy.yaml
> > create mode 100644 drivers/media/platform/ti-vpe/ti-csi2rx.c
--
Regards,
Laurent Pinchart
Hi Jacopo,
On Wed, Mar 17, 2021 at 11:14:12AM +0100, Jacopo Mondi wrote:
> On Tue, Mar 16, 2021 at 12:15:16AM +0200, Laurent Pinchart wrote:
> > On Mon, Mar 15, 2021 at 05:30:25PM +0100, Jacopo Mondi wrote:
> > > The MAX9286 GMSL deserializer features gpio controller capabil
gt; Reviewed-by: Laurent Pinchart
> Cc: Jonas Karlman
> Cc: Jernej Skrabec
> Cc: Sean Paul
> Acked-by: Sam Ravnborg
> Signed-off-by: Stephen Boyd
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20
> 1 file changed, 20 insertions(+)
>
> diff --git
Tested-by: Hans Verkuil
> Signed-off-by: Arnd Bergmann
v4l2_event vs. v4l2_event32 vs. v4l2_event_time32 vs.
v4l2_event32_time32 is a bit confusing. Do I understand correctly that
the code below runs for the non-compat path, thus native userspace
(32-bit on 32-bit machines, 64-bit on 64-bit ma
n leak.
>
> To be on the safe side, always clear the entire ioctl buffer before
> calling the conversion handler functions that are meant to initialize
> them.
>
> Signed-off-by: Arnd Bergmann
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/v4l2-core/v4l2-ioct
gt; list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
> if (iter->funcs->pre_enable)
> iter->funcs->pre_enable(iter);
> +
> + if (iter == bridge)
> + break;
This looks
Hi Doug,
Thank you for the patch.
On Fri, Apr 02, 2021 at 03:28:37PM -0700, Douglas Anderson wrote:
> A random comment inside a function had "/**" in front of it. That
> doesn't make sense. Remove.
>
> Signed-off-by: Douglas Anderson
> Reviewed-by: Andrzej Hajda
move us closer to
> a proper remove.
>
> Signed-off-by: Douglas Anderson
> Reviewed-by: Andrzej Hajda
Reviewed-by: Laurent Pinchart
> ---
>
> Changes in v3:
> - Removed "NOTES" from commit message.
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 15 +
() to make sure things are powered on (and then off again)
> when reading the EDID.
>
> Signed-off-by: Douglas Anderson
> Reviewed-by: Andrzej Hajda
Reviewed-by: Laurent Pinchart
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++--
>
> Reviewed-by: Andrzej Hajda
Reviewed-by: Laurent Pinchart
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 12
> 1 file changed, 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/driver
reason for us
> to call it again.
>
> Signed-off-by: Douglas Anderson
> Reviewed-by: Andrzej Hajda
Reviewed-by: Laurent Pinchart
This looks like a widespread issue, would you be able to send a patch to
address all the other drivers ?
> ---
>
> (no changes since v1)
>
he bridge chip without the "refclk" they're
> in no worse shape than they were before the (fairly recent) commit
> 58074b08c04a ("drm/bridge: ti-sn65dsi86: Read EDID blob over DDC").
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Laurent Pinchart
> ---
&g
y:
> - Putting an error message in the logs if we fall back.
> - Putting a comment in saying what's going on.
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Laurent Pinchart
> ---
>
> (no changes since v1)
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 7 ++
working with - when the drm_bridge is attached.
> Likewise, we unregister the AUX adapter on bridge detachment by adding a
> ti_sn_bridge_detach() callback.
>
> Signed-off-by: Lyude Paul
Reviewed-by: Laurent Pinchart
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 18 +
@name: user-visible name of this AUX channel and the I2C-over-AUX adapter
> * @ddc: I2C adapter that can be used for I2C-over-AUX communication
> * @dev: pointer to struct device that is the parent for this AUX channel
> + * @drm_dev: pointer to the &drm_device that owns this AUX channel. Beware,
> this may be %NULL
> + * before drm_dp_aux_register() has been called.
> * @crtc: backpointer to the crtc that is currently using this AUX channel
> * @hw_mutex: internal mutex used for locking transfers
> * @crc_work: worker that captures CRCs for each frame
> * @crc_count: counter of captured frame CRCs
> * @transfer: transfers a message representing a single AUX transaction
> *
> - * The @dev field should be set to a pointer to the device that implements
> the AUX channel.
> + * The @dev field should be set to a pointer to the device that implements
> the AUX channel. As well,
> + * the @drm_dev field should be set to the &drm_device that will be using
> this AUX channel as early
> + * as possible. For many graphics drivers this should happen before
> drm_dp_aux_init(), however it's
> + * perfectly fine to set this field later so long as it's assigned before
> calling
> + * drm_dp_aux_register().
> *
> * The @name field may be used to specify the name of the I2C adapter. If
> set to %NULL, dev_name()
> * of @dev will be used.
> @@ -1866,6 +1872,7 @@ struct drm_dp_aux {
> const char *name;
> struct i2c_adapter ddc;
> struct device *dev;
> + struct drm_device *drm_dev;
> struct drm_crtc *crtc;
> struct mutex hw_mutex;
> struct work_struct crc_work;
--
Regards,
Laurent Pinchart
Hi Lyude,
Thank you for the patch.
On Fri, Feb 19, 2021 at 04:53:15PM -0500, Lyude Paul wrote:
> So that we can start using drm_dbg_*() in
> drm_dp_link_train_clock_recovery_delay().
>
> Signed-off-by: Lyude Paul
Reviewed-by: Laurent Pinchart
> ---
> drivers/
Hi Lyude,
Thank you for the patch.
On Fri, Feb 19, 2021 at 04:53:16PM -0500, Lyude Paul wrote:
> So that we can start using drm_dbg_*() for
> drm_dp_link_train_channel_eq_delay() and
> drm_dp_lttpr_link_train_channel_eq_delay().
>
> Signed-off-by: Lyude Paul
Reviewed-by: L
Hi Andrey,
Thank you for the patch.
On Thu, Feb 18, 2021 at 08:16:40PM +0300, Andrey Konovalov wrote:
> Print a warning if V4L2_CID_LINK_FREQ control is not implemented.
>
> Signed-off-by: Andrey Konovalov
> Reviewed-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
&
the flag names, e.g.
> MIPI_DSI_MODE_NO_EOT_PACKET.
>
> Signed-off-by: Nicolas Boichat
This looks good to me, it increases readability.
Reviewed-by: Laurent Pinchart
Please however see the end of the mail for a comment.
> ---
> I considered adding _DISABLE_ instead, but that&#
(0x88 + (0x20 *
> (i)))
>
> -#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * i))
> +#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * (i)))
> #define CSI2_CTX_CTRL3_ALPHA_SHIFT 5
> #define CSI2_CTX_CTRL3_ALPHA_MASK\
> (0x3fff << CSI2_CTX_CTRL3_ALPHA_SHIFT)
--
Regards,
Laurent Pinchart
parentheses in a
subsequent patch if desired.
--
Regards,
Laurent Pinchart
s.h
>
> Signed-off-by: Nikolay Kyx
Reviewed-by: Laurent Pinchart
> ---
>
> Additionally some style warnings remain valid here and could be fixed by
> another patch.
>
> drivers/staging/media/omap4iss/iss_regs.h | 16
> 1 file changed, 8 insertions(+), 8
tch must also be accompanied by the
> update in max9286 somehow?
>
> I guess we can't keep 'test bisectability' with this very easily so it
> probably doesn't matter too much, the end result will be the interesting
> part.
>
> Reviewed-by: Kieran Bingham
>
> > }
> >
> > static int rdacm20_probe(struct i2c_client *client)
--
Regards,
Laurent Pinchart
ral rule, peek into the
max9271_device structure directly. It may be nice to add a
max9271_init() function that will initialize the client field, and move
the client->addr assignment to max9271_set_address().
Maybe you've already done so in the rest of the series, I'll find out
soon :-) For
Hi Jacopo,
Reviewed-by: Laurent Pinchart
On Wed, Feb 17, 2021 at 01:01:26PM +, Kieran Bingham wrote:
> On 16/02/2021 17:41, Jacopo Mondi wrote:
> > During the camera module initialization the image sensor PID is read to
> > verify it can correctly be identified. The current
tic change only.
>
> Signed-off-by: Jacopo Mondi
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/rdacm20.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c
> index 6504ed0bd3bc..56
CSLVSH_469NS_234NS |
> + MAX9271_I2CSLVTO_1024US |
> + MAX9271_I2CMSTBT_105KBPS);
> + if (ret)
> + return ret;
>
> - max9271_configure_gmsl_link(&dev->serializer);
> + ret = max9271_configure_gmsl_link(&dev->serialize
set and initialize it. */
> ret = max9271_set_gpios(&dev->serializer, MAX9271_GPIO1OUT);
> if (ret)
> return ret;
> - usleep_range(1, 15000);
Maybe a comment here to state that the delay has to be at least 2048
XVCLK cycles would be useful ?
With
ile too, and the RDACM's shouldn't care about it.
I think this is a good idea. With this addressed,
Reviewed-by: Laurent Pinchart
> If we end up moving the max9271 'library' into more of a module/device
> then this would have to be done in it's 'probe'
(chan));
> + max9286_i2c_mux_configure(priv, MAX9286_FWDCCEN(chan) |
> + MAX9286_REVCCEN(chan));
I feel obliged to say I would have written
max9286_i2c_mux_configure(priv, MAX9286_FWDCCEN(chan) |
MAX9286
priv->reverse_channel_mv = reverse_channel_microvolt / 1000U;
> + priv->init_rev_chan_mv = reverse_channel_microvolt / 1000U;
>
> priv->route_mask = priv->source_mask;
>
--
Regards,
Laurent Pinchart
orm additional tests after fixing this.
> usleep_range(3000, 5000);
>
> /* Read OV10640 ID to test communications. */
--
Regards,
Laurent Pinchart
ondi
Reviewed-by: Laurent Pinchart
> ---
> drivers/media/i2c/max9286.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index 1f14cd817fbf..4afb5ca06448 100644
> --- a
channel amplitude to compensate for the
> - * remote ends high threshold, if not done already
> + * remote ends high threshold
I would have done this in the previous patch, but it doesn't matter
much.
Reviewed-by: Laurent Pinchart
>* - Verify all configuratio
Hi Jacopo,
Reviewed-by: Laurent Pinchart
On Thu, Feb 18, 2021 at 04:13:14PM +, Kieran Bingham wrote:
> On 16/02/2021 17:41, Jacopo Mondi wrote:
> > The current probe() procedure of the RDACM20 and RDACM20 performs
>
> and RDACM21?
>
> > initialization of the seri
tected
>* - Disable auto-ack as communication on the control channel are now
>* stable.
>*/
> - max9286_reverse_channel_setup(priv, MAX9286_REV_AMP_HIGH);
> max9286_check_config_link(priv, priv->source_mask);
>
> /*
--
Regards,
Laurent Pinchart
> > index 80b6f16f87a8..552985026458 100644
> > --- a/drivers/media/i2c/rdacm21.c
> > +++ b/drivers/media/i2c/rdacm21.c
> > @@ -442,7 +442,7 @@ static int rdacm21_init(struct v4l2_subdev *sd,
> > unsigned int val)
> > ret = max9271_configure_i2c(&dev->serializer,
> > MAX9271_I2CSLVSH_469NS_234NS |
> > MAX9271_I2CSLVTO_1024US |
> > - MAX9271_I2CMSTBT_105KBPS);
> > + MAX9271_I2CMSTBT_339KBPS);
> > if (ret)
> > return ret;
> >
--
Regards,
Laurent Pinchart
r to do so in the
operations themselves, for instance replacing
list_for_each_entry_reverse() with list_for_each_entry() in
drm_atomic_bridge_chain_pre_enable(). Still, this will likely break
drivers that depend on the existing order, so I don't think that's an
acceptable solution as-is.
>
> if (bridge->funcs->attach) {
> ret = bridge->funcs->attach(bridge, flags);
--
Regards,
Laurent Pinchart
Hi Enrico,
On Wed, Dec 09, 2020 at 12:11:36PM +0100, Enrico Weigelt, metux IT consult
wrote:
> On 08.12.20 16:54, Laurent Pinchart wrote:
> >> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
> >> b/drivers/usb/gadget/udc/atmel_usba_udc.c
> >> index 2b8
ilability of a device that doesn't
> actually exist, so if the check isn't implemented just assume that the
> "device" is present.
>
> Suggested-by: Laurent Pinchart
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since
d support to fwnode_graph_get_endpoint_by_id() to
> find those endpoints by recursively calling itself passing the ptr to
> fwnode->secondary in the event no endpoint is found for the primary.
>
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since RFC v3
fix that.
>
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since RFC v3:
>
> Put reference to previous child.
>
> drivers/base/swnode.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bas
}
> +
> ret = software_node_register(&nodes[i]);
> - if (ret) {
> - software_node_unregister_nodes(nodes);
> - return ret;
> - }
> + if (ret)
> + go
On Mon, Nov 30, 2020 at 06:11:52PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Mon, Nov 30, 2020 at 01:31:15PM +, Daniel Scally wrote:
> > Registering software_nodes with the .parent member set to point to a
> > currently unregis
to child, reverse the order in which
> this function unregisters software_nodes.
>
> Suggested-by: Andy Shevchenko
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since RFC v3:
>
> Switched this functionality from a new function
cally
I"d squash this with the previous patch to avoid introducing an
inconsistency.
Reviewed-by: Laurent Pinchart
> ---
> Changes since v3:
>
> Patch introduced
>
> drivers/base/swnode.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>
t; .put = software_node_put,
> @@ -551,7 +655,11 @@ static const struct fwnode_operations software_node_ops
> = {
> .get_parent = software_node_get_parent,
> .get_next_child_node = software_node_get_next_child,
> .get_named_child_node = software_node_get_named_child_node,
> - .get_reference_args = software_node_get_reference_args
> + .get_reference_args = software_node_get_reference_args,
> + .graph_get_next_endpoint = software_node_graph_get_next_endpoint,
> + .graph_get_remote_endpoint = software_node_graph_get_remote_endpoint,
> + .graph_get_port_parent = software_node_graph_get_port_parent,
> + .graph_parse_endpoint = software_node_graph_parse_endpoint,
> };
>
> /*
> -- */
--
Regards,
Laurent Pinchart
y: Andy Shevchenko
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since RFC v3:
>
> Changed the called function name - didn't drop the tags since it's
> such a trivial change, hope that's alright!
>
> lib/test_printf.
match against it during match_fwnode() to
> accommodate that possibility.
>
> Signed-off-by: Daniel Scally
Reviewed-by: Laurent Pinchart
> ---
> Changes since RFC v3:
>
> - None
>
> drivers/media/v4l2-core/v4l2-async.c | 8
> 1 file changed, 8 ins
t; +
> +#endif
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 36e354ecf71e..0d69b593e9f0 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1702,6 +1702,22 @@ static void cio2_queues_exit(struct cio2_device *cio2)
> cio2_queue_exit(cio2, &cio2->queue[i]);
> }
>
> +static bool cio2_check_fwnode_graph(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *endpoint;
> +
> + if (IS_ERR_OR_NULL(fwnode))
> + return false;
> +
> + endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
> + if (endpoint) {
> + fwnode_handle_put(endpoint);
> + return true;
> + }
> +
> + return cio2_check_fwnode_graph(fwnode->secondary);
If we have a fwnode->secondary and this check fails there's something
seriously wrong, I wonder if we should print an error message.
Overall this is nice. I think the next version will get my ack :-)
> +}
> +
> / PCI interface /
>
> static int cio2_pci_probe(struct pci_dev *pci_dev,
> @@ -1715,6 +1731,17 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
> return -ENOMEM;
> cio2->pci_dev = pci_dev;
>
> + /*
> + * On some platforms no connections to sensors are defined in firmware,
> + * if the device has no endpoints then we can try to build those as
> + * software_nodes parsed from SSDB.
> + */
> + if (!cio2_check_fwnode_graph(dev_fwnode(&pci_dev->dev))) {
> + r = cio2_bridge_init(pci_dev);
> + if (r)
> + return r;
> + }
> +
> r = pcim_enable_device(pci_dev);
> if (r) {
> dev_err(&pci_dev->dev, "failed to enable device (%d)\n", r);
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> index ccf0b85ae36f..520a27c9cdad 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> @@ -437,4 +437,10 @@ static inline struct cio2_queue
> *vb2q_to_cio2_queue(struct vb2_queue *vq)
> return container_of(vq, struct cio2_queue, vbq);
> }
>
> +#if IS_ENABLED(CONFIG_CIO2_BRIDGE)
> +int cio2_bridge_init(struct pci_dev *cio2);
> +#else
> +int cio2_bridge_init(struct pci_dev *cio2) { return 0; }
> +#endif
> +
> #endif
--
Regards,
Laurent Pinchart
new_device(struct device *dev,
> {
> return ERR_PTR(-ENODEV);
> }
> +static inline char *i2c_acpi_dev_name(struct acpi_device *adev)
> +{
> + return NULL;
> +}
> static inline struct i2c_adapter
> *i2c_acpi_find_adapter_by_handle(acpi_handle handle)
> {
> return NULL;
--
Regards,
Laurent Pinchart
v) {
> - dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
> + dev_set_name(&client->dev, i2c_acpi_dev_name(adev));
> return;
> }
>
--
Regards,
Laurent Pinchart
dummy INT3472 device upon which the sensor depends. This
> > driver binds to the dummy acpi device (which does not represent a
> > physical PMIC) and maps them into GPIO lines and regulators for use by
> > the sensor device instead.
> >
> > Suggested-by:
the
> secondary in cases where it's not already done.
We could also move the !fwnode check to the beginning of the function.
> > }
> > EXPORT_SYMBOL_GPL(fwnode_graph_get_endpoint_by_id);
--
Regards,
Laurent Pinchart
}
> > +
> > + if (candidate == dependee) {
> > + acpi_dev_put(candidate);
> > + kfree(info);
> > + return 1;
> > + }
> > +
> > + kfree(info);
> > + }
> > + }
>
> Can you utilize (by moving to here and export for ACPI layer the
> acpi_lpss_dep()?
--
Regards,
Laurent Pinchart
Hi Andy,
On Mon, Nov 30, 2020 at 07:53:19PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 30, 2020 at 07:28:57PM +0200, Laurent Pinchart wrote:
> > On Mon, Nov 30, 2020 at 07:29:00PM +0200, Andy Shevchenko wrote:
> > > On Mon, Nov 30, 2020 at 01:31:13PM +, D
list_head list;
> >> +};
> >> +
> >> +struct int3472_sensor_regulator_map {
> >> + char *sensor_module_name;
> >> + unsigned int n_supplies;
> >> + struct regulator_consumer_supply *supplies;
> >> +};
> >> +
> >> +/*
> >> + * Here follows platform specific mapping information that we can pass to
> >> + * regulator_init_data when we register our regulators. They're just
> >> mapped
> >> + * via index, I.E. the first regulator pin that the code finds for the
> >> + * i2c-OVTI2680:00 device is avdd, the second is dovdd and so on.
> >> + */
> >> +
> >> +static struct regulator_consumer_supply miix_510_ov2680[] = {
> >> + { "i2c-OVTI2680:00", "avdd" },
> >> + { "i2c-OVTI2680:00", "dovdd" },
> >> +};
> >> +
> >> +static struct regulator_consumer_supply surface_go2_ov5693[] = {
> >> + { "i2c-INT33BE:00", "avdd" },
> >> + { "i2c-INT33BE:00", "dovdd" },
> >> +};
> >> +
> >> +static struct regulator_consumer_supply surface_book_ov5693[] = {
> >> + { "i2c-INT33BE:00", "avdd" },
> >> + { "i2c-INT33BE:00", "dovdd" },
> >> +};
> >> +
> >> +static struct int3472_sensor_regulator_map
> >> int3472_sensor_regulator_maps[] = {
> >> + { "GNDF140809R", 2, miix_510_ov2680 },
> >> + { "YHCU", 2, surface_go2_ov5693 },
> >> + { "MSHW0070", 2, surface_book_ov5693 },
> >> +};
--
Regards,
Laurent Pinchart
e, like
>
> /* 79234640-9e10-4fea-a5c1b5aa8b19756f */
> const guid_t int3472_gpio_guid =
> GUID_INIT(0x79234640, 0x9e10, 0x4fea,
> 0xa5, 0xc1, 0xb5, 0xaa, 0x8b, 0x19, 0x75, 0x6f);
>
> ...
>
> > +static struct regulator_consumer_supply miix_510_ov2680[] = {
> > + { "i2c-OVTI2680:00", "avdd" },
> > + { "i2c-OVTI2680:00", "dovdd" },
> > +};
>
> Can we use acpi_dev_first_match_dev() to get instance name out of their HIDs?
>
> > +static struct regulator_consumer_supply surface_go2_ov5693[] = {
> > + { "i2c-INT33BE:00", "avdd" },
> > + { "i2c-INT33BE:00", "dovdd" },
> > +};
> > +
> > +static struct regulator_consumer_supply surface_book_ov5693[] = {
> > + { "i2c-INT33BE:00", "avdd" },
> > + { "i2c-INT33BE:00", "dovdd" },
> > +};
>
> Ditto.
>
> ...
>
> > +static struct int3472_sensor_regulator_map int3472_sensor_regulator_maps[]
> > = {
> > + { "GNDF140809R", 2, miix_510_ov2680 },
> > + { "YHCU", 2, surface_go2_ov5693 },
> > + { "MSHW0070", 2, surface_book_ov5693 },
> > +};
>
> Hmm... Usual way is to use DMI for that. I'm not sure above will not give us
> false positive matches.
--
Regards,
Laurent Pinchart
ESET_USB0_HIBERRESET>,
> + <&zynqmp_reset ZYNQMP_RESET_USB0_APB>;
> + reset-names = "usb0_crst", "usb0_hibrst", "usb0_apbrst";
> };
>
> usb1: usb@fe30 {
> @@ -743,6 +768,10 @@ usb1: usb@fe30 {
> reg = <0x0 0xfe30 0x0 0x4>;
> clock-names = "clk_xin", "clk_ahb";
> power-domains = <&zynqmp_firmware PD_USB_1>;
> + resets = <&zynqmp_reset ZYNQMP_RESET_USB1_CORERESET>,
> + <&zynqmp_reset ZYNQMP_RESET_USB1_HIBERRESET>,
> + <&zynqmp_reset ZYNQMP_RESET_USB1_APB>;
> + reset-names = "usb1_crst", "usb1_hibrst", "usb1_apbrst";
> };
>
> watchdog0: watchdog@fd4d {
--
Regards,
Laurent Pinchart
On Mon, Dec 07, 2020 at 12:46:56AM +0200, Laurent Pinchart wrote:
> Hi Michal,
>
> Thank you for the patch.
>
> On Wed, Dec 02, 2020 at 03:06:05PM +0100, Michal Simek wrote:
> > Add label which is used by bootloader for adding bootloader specific flag.
> >
>
901 - 1000 of 2684 matches
Mail list logo
