Re: [PATCH] media: v4l2-async: Put fwnode after last access

t; } > > - fwnode_handle_put(dev_fwnode); > + match = (dev_fwnode == other_fwnode); > > - if (dev_fwnode != other_fwnode) This only performs a pointer comparison, it doesn't access dev_fwnode. I don't think the change is necessary. > + fwnode_handle_put(dev_fwnode); > + if (!match) > return false; > > /* -- Regards, Laurent Pinchart

Re: [PATCH v7 08/12] media: uvcvideo: Use dev_ printk aliases

\ > > + dev_info(&(_dev)->udev->dev, fmt, ##__VA_ARGS__); \ > > +} while (0) > > ... > > Why not to use dev_warn_once() instead? uvc_warn_once() prints the warning once per device, not once globally. -- Regards, Laurent Pinchart

Re: [PATCH v3 13/14] include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type

CSI-1 bus > + * @V4L2_FWNODE_BUS_TYPE_CCP2: SMIA Compact Camera Port 2 bus > + * @V4L2_FWNODE_BUS_TYPE_CSI2_DPHY: MIPI CSI-2 bus, D-PHY physical layer > + * @V4L2_FWNODE_BUS_TYPE_PARALLEL: Camera Parallel Interface bus > + * @V4L2_FWNODE_BUS_TYPE_BT656: BT656 video format

Re: [PATCH v3 07/14] software_node: Add support for fwnode_graph*() family of functions

ot;port@") and let the compiler optimize this to a compile-time constant. > > + 10, >port); > > + if (ret) > > + return ret; > > + > > + endpoint->id = swnode->id; > > + endpoint->local_fwnode = fwnode; > > + > > + return 0; > > +} > > + > > static const struct fwnode_operations software_node_ops = { > > .get = software_node_get, > > .put = software_node_put, > > @@ -551,7 +657,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

Re: [PATCH v3 07/14] software_node: Add support for fwnode_graph*() family of functions

->parent; > + > + return swnode ? software_node_get(>fwnode) : NULL; > +} > + > +static int > +software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode, > +struct fwnode_endpoint *endpoint) > +{ > + struct swnode *swnod

Re: [PATCH v3 06/14] include: fwnode.h: Define format macros for ports and endpoints

; _FORMAT -> _FMT (however, V4L2 guys may correct me, because IIRC _FMT > suffix is also used for other things in v4l2. This isn't related to V4L2, so it doesn't matter much :-) I personally prefer spelling names out in full when that wouldn't result in too long lines, but it's really a matter of personal preference, I don't mind either way. Reviewed-by: Laurent Pinchart > > #define NR_FWNODE_REFERENCE_ARGS 8 > > > > /** -- Regards, Laurent Pinchart

Re: [PATCH] dt-bindings: Drop redundant maxItems/items

ndant. 'maxItems' is > > > preferred for a single entry while greater than 1 should have an 'items' > > > list. > > > > > > A meta-schema check for this is pending once these existing cases are > > > fixed. > > > > > > Cc: Laurent Pinch

Re: [PATCH v6 06/11] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

@@ -56,6 +59,9 @@ > #define UVC_GUID_UVC_SELECTOR \ > {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ >0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02} > +#define UVC_GUID_EXT_GPIO_CONTROLLER \ > + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03} > > #define UVC_GUID_FORMAT_MJPEG \ > { 'M', 'J', 'P', 'G', 0x00, 0x00, 0x10, 0x00, \ > @@ -213,6 +219,7 @@ > */ > > struct uvc_device; > +struct gpio_desc; Alphabetical order ? :-) Otherwise this looks good, pending an agreement on how to handle the case where the GPIO can't generate an IRQ. > > /* TODO: Put the most frequently accessed fields at the beginning of > * structures to maximize cache efficiency. > @@ -353,6 +360,14 @@ struct uvc_entity { > u8 *bmControls; > u8 *bmControlsType; > } extension; > + > + struct { > + u8 bControlSize; > + u8 *bmControls; > + struct gpio_desc *gpio_privacy; > + int irq; > + atomic_t gpio_privacy_value; > + } gpio; > }; > > u8 bNrInPins; > @@ -690,6 +705,8 @@ struct uvc_device { > struct uvc_control *ctrl; > const void *data; > } async_ctrl; > + > + struct uvc_entity *gpio_unit; > }; > > enum uvc_handle_state { -- Regards, Laurent Pinchart

Re: [PATCH v6 04/11] media: uvcvideo: Add uvc_ctrl_status_event_direct

> bool uvc_ctrl_status_event(struct urb *urb, struct uvc_video_chain *chain, > struct uvc_control *ctrl, const u8 *data); > +void uvc_ctrl_status_event_direct(struct uvc_video_chain *chain, > + struct uvc_control *ctrl, const u8 *data); > > int uvc_ctrl_begin(struct uvc_video_chain *chain); > int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback, -- Regards, Laurent Pinchart

Re: [PATCH v5 09/12] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM

Hi Ricardo, On Tue, Dec 22, 2020 at 09:04:19PM +0100, Ricardo Ribalda wrote: > On Tue, Dec 22, 2020 at 11:30 AM Laurent Pinchart wrote: > > On Mon, Dec 21, 2020 at 05:48:16PM +0100, Ricardo Ribalda wrote: > > > Some devices, can only read the privacy_pin if the device is

Re: [PATCH v5 07/12] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

Hi Ricardo, On Tue, Dec 22, 2020 at 07:36:52PM +0100, Ricardo Ribalda wrote: > On Tue, Dec 22, 2020 at 9:34 AM Laurent Pinchart wrote: > > On Mon, Dec 21, 2020 at 05:48:14PM +0100, Ricardo Ribalda wrote: > > > Some devices can implement a physical switch to disable the input o

Re: [PATCH v6] drm/bridge: add it6505 driver

-Chih Tseng (曾昭智); Hermes Wu (吳佳宏); Kenneth Hung (洪家倫); Allen Chen > (陳柏宇); Pi-Hsun Shih; Jitao Shi; Yilun Lin; Hermes Wu (吳佳宏); Andrzej Hajda; > Neil Armstrong; Laurent Pinchart; Jonas Karlman; Jernej Skrabec; David > Airlie; Daniel Vetter; Matthias Brugger; open list; open list:DRM DRIVERS;

Re: [PATCH v5 09/12] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM

209,7 @@ > #define UVC_QUIRK_RESTORE_CTRLS_ON_INIT 0x0400 > #define UVC_QUIRK_FORCE_Y8 0x0800 > #define UVC_QUIRK_FORCE_BPP 0x1000 > +#define UVC_QUIRK_PRIVACY_DURING_STREAM 0x2000 > > /* Format flags */ > #define UVC_FMT_FLAG_COMPRESSED 0x0001 > @@ -826,6 +827,9 @@ extern const struct v4l2_file_operations uvc_fops; > int uvc_mc_register_entities(struct uvc_video_chain *chain); > void uvc_mc_cleanup_entity(struct uvc_entity *entity); > > +/* Privacy gpio */ > +void uvc_privacy_gpio_event(struct uvc_device *dev); > + > /* Video */ > int uvc_video_init(struct uvc_streaming *stream); > int uvc_video_suspend(struct uvc_streaming *stream); -- Regards, Laurent Pinchart

Re: [PATCH v5 12/12] media: uvcvideo: use dev_dbg() for uvc_trace()

output. > > Now that we are at it, regroup the strings so the messages can be easily > "grepable". > > Suggested-by: Laurent Pinchart > Signed-off-by: Ricardo Ribalda > --- > drivers/media/usb/uvc/uvc_ctrl.c | 66 --- >

Re: [PATCH v5 07/12] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

On Tue, Dec 22, 2020 at 10:33:52AM +0200, Laurent Pinchart wrote: > Hi Ricardo, > > Thank you for the patch. > > On Mon, Dec 21, 2020 at 05:48:14PM +0100, Ricardo Ribalda wrote: > > Some devices can implement a physical switch to disable the input of the > > camera

Re: [PATCH v5 07/12] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

+#include > #include > #include > #include > @@ -37,6 +38,8 @@ > (UVC_ENTITY_IS_TERM(entity) && \ > ((entity)->type & 0x8000) == UVC_TERM_OUTPUT) > > +#define UVC_EXT_GPIO_UNIT0x7ffe > +#define UVC_EXT_GPIO_UNIT_ID 0x100 > > /* > * GUIDs > @@ -56,6 +59,9 @@ > #define UVC_GUID_UVC_SELECTOR \ > {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ >0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02} > +#define UVC_GUID_EXT_GPIO_CONTROLLER \ > + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03} > > #define UVC_GUID_FORMAT_MJPEG \ > { 'M', 'J', 'P', 'G', 0x00, 0x00, 0x10, 0x00, \ > @@ -278,6 +284,8 @@ struct uvc_format_desc { > u32 fcc; > }; > > +struct gpio_desc; > + This could be moved a bit up, with the other forward declaration. > /* The term 'entity' refers to both UVC units and UVC terminals. > * > * The type field is either the terminal type (wTerminalType in the terminal > @@ -353,6 +361,14 @@ struct uvc_entity { > u8 *bmControls; > u8 *bmControlsType; > } extension; > + > + struct { > + u8 bControlSize; > + u8 *bmControls; > + struct gpio_desc *gpio_privacy; > + int irq; > + atomic_t gpio_privacy_value; > + } gpio; > }; > > u8 bNrInPins; -- Regards, Laurent Pinchart

Re: [PATCH v5 02/12] media: uvcvideo: Allow more that one asyc_ctrl

struct input_dev *input; > char input_phys[64]; > > - struct uvc_ctrl_work { > - struct work_struct work; > - struct urb *urb; > - struct uvc_video_chain *chain; > - struct uvc_control *ctrl; > - u8 data[UVC_MAX_STATUS_SIZE]; > - } async_ctrl; > + /* Async control */ > + struct work_struct async_ctrl_work; > + struct list_head async_ctrl_list; > + struct mutex async_ctrl_lock; > }; > > enum uvc_handle_state { -- Regards, Laurent Pinchart

Re: [PATCH v5 01/12] media: uvcvideo: Fix race condition handling events

b/drivers/media/usb/uvc/uvcvideo.h > @@ -678,7 +678,7 @@ struct uvc_device { > struct urb *urb; > struct uvc_video_chain *chain; > struct uvc_control *ctrl; > - const void *data; > + u8 data[UVC_MAX_STATUS_SIZE]; > } async_ctrl; > }; > -- Regards, Laurent Pinchart

Re: [PATCH 11/14] dt-bindings: display: bridge: Add i.MX8qm/qxp LVDS display bridge binding

Hi Liu, On Tue, Dec 22, 2020 at 09:36:37AM +0200, Laurent Pinchart wrote: > On Thu, Dec 17, 2020 at 05:59:30PM +0800, Liu Ying wrote: > > This patch adds bindings for i.MX8qm/qxp LVDS display bridge(LDB). > > > > Signed-off-by: Liu Ying > > --- > > .../bi

Re: [PATCH 11/14] dt-bindings: display: bridge: Add i.MX8qm/qxp LVDS display bridge binding

annel@0 { > +#address-cells = <1>; > +#size-cells = <0>; > +reg = <0>; > +phys = <_lvds_0_phy>; > +phy-names = "lvds_phy"; > + > +port@0 { > + reg = <0>; > + > +mipi_lvds_0_ldb_ch0_mipi_lvds_0_pxl2dpi: endpoint { > +remote-endpoint = > <_lvds_0_pxl2dpi_mipi_lvds_0_ldb_ch0>; > +}; > +}; > +}; > + > +channel@1 { > +#address-cells = <1>; > +#size-cells = <0>; > +reg = <1>; > +phys = <_lvds_0_phy>; > +phy-names = "lvds_phy"; > + > +port@0 { > +reg = <0>; > + > +mipi_lvds_0_ldb_ch1_mipi_lvds_0_pxl2dpi: endpoint { > +remote-endpoint = > <_lvds_0_pxl2dpi_mipi_lvds_0_ldb_ch1>; > +}; > +}; > +}; > +}; -- Regards, Laurent Pinchart

Re: [PATCH 06/14] dt-bindings: display: bridge: Add i.MX8qm/qxp display pixel link binding

lls = <1>; > +#size-cells = <0>; > +reg = <1>; > + > +dc0_pixel_link0_mipi_lvds_0_pxl2dpi: endpoint@0 { > + reg = <0>; > +remote-endpoint = <_lvds_0_pxl2dpi_dc0_pixel_link0>; > +}; > + > +dc0_pixel_link0_mipi_lvds_1_pxl2dpi: endpoint@1 { > +reg = <1>; > +remote-endpoint = <_lvds_1_pxl2dpi_dc0_pixel_link0>; > +}; > +}; > + > +/* to imaging subsystem */ > +port@4 { > +reg = <4>; > +}; > +}; > +}; -- Regards, Laurent Pinchart

Re: [PATCH] dt-bindings: Drop unnecessary *-supply schemas properties

fixed. > > Cc: Jonathan Cameron > Cc: Dmitry Torokhov > Cc: Laurent Pinchart > Cc: Mauro Carvalho Chehab > Cc: Sakari Ailus > Cc: Lee Jones > Cc: Mark Brown > Cc: Maxime Ripard > Cc: dri-de...@lists.freedesktop.org > Cc: linux-...@vger.kernel.org

Re: [PATCH] dt-bindings: Drop redundant maxItems/items

this is pending once these existing cases are > fixed. > > Cc: Laurent Pinchart > Cc: Vinod Koul > Cc: Mark Brown > Cc: Greg Kroah-Hartman > Cc: Jassi Brar > Cc: dri-de...@lists.freedesktop.org > Cc: dmaeng...@vger.kernel.org > Cc: alsa-de...@alsa-project.org

Re: [PATCH v4 9/9] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM

Hi Ricardo, On Mon, Dec 21, 2020 at 02:10:18AM +0100, Ricardo Ribalda wrote: > On Sun, Dec 20, 2020 at 6:22 PM Laurent Pinchart wrote: > > On Tue, Dec 15, 2020 at 04:44:39PM +0100, Ricardo Ribalda wrote: > > > Some devices, can only read the privacy_pin if the device

Re: [PATCH v4 5/9] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

Hi Ricardo, On Sun, Dec 20, 2020 at 11:56:12PM +0100, Ricardo Ribalda wrote: > On Sun, Dec 20, 2020 at 5:49 PM Laurent Pinchart wrote: > > On Tue, Dec 15, 2020 at 04:44:35PM +0100, Ricardo Ribalda wrote: > > > Some devices can implement a physical switch to disable the input o

Re: [PATCH] drm: bridge: adv7511: Remove redundant null check before clk_disable_unprepare

Hi Xu Wang, Thank you for the patch. On Fri, Nov 27, 2020 at 09:18:29AM +, Xu Wang wrote: > Because clk_disable_unprepare() already checked NULL clock parameter, > so the additional check is unnecessary, just remove them. > > Signed-off-by: Xu Wang Reviewed-by: Laur

Re: [PATCH] drm: bridge: dw-hdmi: Remove redundant null check before clk_disable_unprepare

Hi Xu Wang, Thank you for the patch. On Fri, Nov 27, 2020 at 09:23:32AM +, Xu Wang wrote: > Because clk_disable_unprepare() already checked NULL clock parameter, > so the additional check is unnecessary, just remove them. > > Signed-off-by: Xu Wang Reviewed-by: Laur

Re: [PATCH v4 9/9] media: uvcvideo: Implement UVC_QUIRK_PRIVACY_DURING_STREAM

_FORCE_Y8 0x0800 > #define UVC_QUIRK_FORCE_BPP 0x1000 > +#define UVC_QUIRK_PRIVACY_DURING_STREAM 0x2000 > > /* Format flags */ > #define UVC_FMT_FLAG_COMPRESSED 0x0001 > @@ -359,6 +361,7 @@ struct uvc_entity { > u8 bControlSize; > u8 *bmControls; > struct gpio_desc *gpio_privacy; > + atomic_t gpio_privacy_value; > } gpio; > }; > > @@ -815,6 +818,9 @@ extern const struct v4l2_file_operations uvc_fops; > int uvc_mc_register_entities(struct uvc_video_chain *chain); > void uvc_mc_cleanup_entity(struct uvc_entity *entity); > > +/* Privacy gpio */ > +void uvc_privacy_gpio_event(struct uvc_device *dev); > + > /* Video */ > int uvc_video_init(struct uvc_streaming *stream); > int uvc_video_suspend(struct uvc_streaming *stream); -- Regards, Laurent Pinchart

Re: [PATCH v4 8/9] media: uvcvideo: New macro uvc_trace_cont

Hi Ricardo, Thank you for the patch. On Tue, Dec 15, 2020 at 04:44:38PM +0100, Ricardo Ribalda wrote: > Remove all the duplicated code around pr_cont with a new macro. > > Suggested-by: Joe Perches > Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart > --- >

Re: [PATCH v4 7/9] media: uvcvideo: Use dev_ printk aliases

uvc_urb_index(uvc_urb), ret); > uvc_video_stop_transfer(stream, 1); > return ret; > } > @@ -1989,7 +1991,8 @@ int uvc_video_init(struct uvc_streaming *stream) > int ret; > > if (stream->nformats == 0) { &

Re: [PATCH v4 6/9] media: uvcvideo: Add Privacy control based on EXT_GPIO

Hi Ricardo, Thank you for the patch. On Tue, Dec 15, 2020 at 04:44:36PM +0100, Ricardo Ribalda wrote: > Add a new control and mapping for Privacy controls connected to > UVC_GUID_EXT_GPIO_CONTROLLERs. > > Signed-off-by: Ricardo Ribalda Reviewed-by: Laurent Pinchart > --- &

Re: [PATCH v4 2/9] media: uvcvideo: Allow external entities

unsigned int flags; > > - u8 id; > + u16 id; /* 0-255: usb entity. 256-65535: external entities */ How about /* * Entities exposed by the UVC device use IDs 0-255, extra entities * implemented by the driver (such as the GPIO entity) use IDs 256 and

Re: [PATCH v4 5/9] media: uvcvideo: Implement UVC_EXT_GPIO_UNIT

(UVC_ENTITY_IS_TERM(entity) && \ > ((entity)->type & 0x8000) == UVC_TERM_OUTPUT) > > +#define UVC_EXT_GPIO_UNIT 0x7ffe > +#define UVC_EXT_GPIO_UNIT_ID 0x100 Maybe a couple of tabs to align this with the other defines above ? > > /* > * GUIDs > @@ -56,6 +59,9 @@ > #define UVC_GUID_UVC_SELECTOR \ > {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ >0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x02} > +#define UVC_GUID_EXT_GPIO_CONTROLLER \ > + {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \ > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03} > > #define UVC_GUID_FORMAT_MJPEG \ > { 'M', 'J', 'P', 'G', 0x00, 0x00, 0x10, 0x00, \ > @@ -348,6 +354,12 @@ struct uvc_entity { > u8 *bmControls; > u8 *bmControlsType; > } extension; > + > + struct { > + u8 bControlSize; > + u8 *bmControls; > + struct gpio_desc *gpio_privacy; > + } gpio; > }; > > u8 bNrInPins; -- Regards, Laurent Pinchart

Re: [PATCH v4 4/9] media: uvcvideo: Entity defined get_info and get_cur

uvc_ctrl_data(ctrl, UVC_CTRL_DATA_CURRENT), > + ctrl->info.size); We could possibly set entity->get_cur by default to a function wrapping uvc_query_ctrl() to avoid the condition here. This isn't mandatory for now though, but I'll t

Re: [PATCH v4 3/9] media: uvcvideo: Allow entities with no pads

if (num_pads) > + num_inputs = (type & UVC_TERM_OUTPUT) ? num_pads : > + num_pads - 1; This is a weird indentation. How about the following ? num_inputs = type & UVC_TERM_OUTPUT ? num_pads : num_pads

Re: [PATCH v4 1/9] media: uvcvideo: Move guid to entity

Hi Ricardo, Thank you for the patch. On Tue, Dec 15, 2020 at 04:44:31PM +0100, Ricardo Ribalda wrote: > Instead of having multiple copies of the entity guid on the code, move > it to the entity structure. > > Reviewed-by: Laurent Pinchart > Signed-off-by: Ricardo Ribalda &

Re: [PATCH 1/1] v4l: ioctl: Fix memory leak in video_usercopy

t_put_array_args(file, user_ptr, > + array_buf, > + array_size, > + orig_cmd, >parg); orig_cmd and

Re: [PATCH v3 1/2] media: dt-bindings: Convert video-interfaces.txt properties to schemas

Hi Rob, On Fri, Dec 18, 2020 at 12:19:55PM -0600, Rob Herring wrote: > On Wed, Dec 16, 2020 at 04:52:20PM +0200, Laurent Pinchart wrote: > > On Thu, Dec 10, 2020 at 03:16:24PM -0600, Rob Herring wrote: > > > Convert video-interfaces.txt to DT schema. As it contains a mixture of

Re: [PATCH v2 12/12] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

Hi Daniel, On Fri, Dec 18, 2020 at 11:57:54PM +, Daniel Scally wrote: > Hi Laurent - thanks for the comments > > On 18/12/2020 16:53, Laurent Pinchart wrote: > >> +static void cio2_bridge_init_property_names(struct cio2_sensor *sensor) > >>

Re: [PATCH v2 12/12] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

}) > + > +#define NODE_PORT(_PORT, _SENSOR_NODE) \ > + ((const struct software_node) { \ > + _PORT, \ > + _SENSOR_NODE, \ > + }) > + > +#define NODE_ENDPOINT(_EP

Re: [PATCH v2 06/12] software_node: Add support for fwnode_graph*() family of functions

.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

Re: [PATCH v2 04/12] software_node: Enforce parent before child ordering of nodes arrays

re set up > + * in any of the software nodes then the array MUST be ordered such that I'd either replace **must** above with MUST, or use **must** here. I'm not sure if kerneldoc handles emphasis with **must**, if it does that seems a bit nicer to me, but it's really up to you. Reviewed-by: Laure

Re: [PATCH] media: vsp1: Fix an error handling path in the probe function

: Add FCP support") > Signed-off-by: Christophe JAILLET Reviewed-by: Laurent Pinchart and queued in my tree for v5.12. > --- > drivers/media/platform/vsp1/vsp1_drv.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/vsp1/vsp

Re: [PATCH v3 2/2] dt-bindings: media: Use graph and video-interfaces schemas

Hi Rob, On Wed, Dec 16, 2020 at 11:38:41AM -0600, Rob Herring wrote: > On Wed, Dec 16, 2020 at 05:19:55PM +0200, Laurent Pinchart wrote: > > On Thu, Dec 10, 2020 at 03:16:25PM -0600, Rob Herring wrote: > > > Now that we have graph and video-interfaces schemas, rework the m

Re: [PATCH v6 5/5] media: i2c: max9286: Configure reverse channel amplitude

, and the desired amplitude after initialization. What do you think ? Is it overkill ? We don't have to implement this now, so Reviewed-by: Laurent Pinchart but if this feature could be required later, we may want to take into account in the naming of the new DT property to reflect the fac

Re: [PATCH v6 2/5] dt-bindings: media: max9286: Document 'maxim,reverse-channel-microvolt'

On Wed, Dec 16, 2020 at 07:05:34PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Dec 15, 2020 at 06:09:54PM +0100, Jacopo Mondi wrote: > > Document the 'reverse-channel-microvolt' vendor property in the > > bindings doc

Re: [PATCH v6 4/5] media: i2c: max9286: Make channel amplitude programmable

On Wed, Dec 16, 2020 at 07:14:25PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Dec 15, 2020 at 06:09:56PM +0100, Jacopo Mondi wrote: > > Instrument the function that configures the reverse channel with a > > p

Re: [PATCH v6 4/5] media: i2c: max9286: Make channel amplitude programmable

press values (100 < x < 130) */ > + chan_amplitude = chan_amplitude < 130 > +? 30 : chan_amplitude - 100; This could also be written chan_amplitude = min(30, chan_amplitude - 100); With or without the change, Revie

Re: [PATCH v6 3/5] media: i2c: max9286: Break-out reverse channel setup

high threshold configuration. > > No functional changes intended. > > Reviewed-by: Kieran Bingham > Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart > --- > drivers/media/i2c/max9286.c | 30 +- > 1 file changed, 17 insertions(+), 13

Re: [PATCH v6 2/5] dt-bindings: media: max9286: Document 'maxim,reverse-channel-microvolt'

; configuration of the GMSL reverse control channel to accommodate > remote serializers pre-programmed with the high threshold power > supply noise immunity enabled. > > Reviewed-by: Kieran Bingham > Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart > --- > v5->v6:

Re: [PATCH v6 1/5] media: i2c: Add driver for RDACM21 camera module

f5a14d7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14809,6 +14809,18 @@ F: drivers/media/i2c/max9271.c > F: drivers/media/i2c/max9271.h > F: drivers/media/i2c/rdacm20.c > > +RDACM21 Camera Sensor > +M: Jacopo Mondi > +M: Kieran Bingham > +M:

Re: [PATCH v3 2/2] dt-bindings: media: Use graph and video-interfaces schemas

Hi Rob, Thank you for the patch. On Thu, Dec 10, 2020 at 03:16:25PM -0600, Rob Herring wrote: > Now that we have graph and video-interfaces schemas, rework the media > related schemas to use them. > > Cc: Maxime Ripard > Cc: Mauro Carvalho Chehab > Cc: Jacopo Mondi >

Re: [PATCH v3 1/2] media: dt-bindings: Convert video-interfaces.txt properties to schemas

Hi Rob, Thank you for the patch. On Thu, Dec 10, 2020 at 03:16:24PM -0600, Rob Herring wrote: > Convert video-interfaces.txt to DT schema. As it contains a mixture of > device level and endpoint properties, split it up into 2 schemas. > > Binding schemas will need to reference both the

Re: [PATCH v2 1/2] media: dt-bindings: Convert video-interfaces.txt properties to schemas

cetree/bindings/media/video-interfaces.txt but that was > > more than 8 years ago, I haven't worked in media / V4L for several years > > now, so, I don't think I can meaningfully maintain that file now. > > Okay, I'll drop you. > > Anyone else want to sign up? Laurent? I'll likely regret this, but yes, you can sign me up :-) -- Regards, Laurent Pinchart

Re: [PATCH] drm: rcar-du: fix reference leak in rcar_cmm_enable

rcar-du: Add support for CMM") > Reported-by: Hulk Robot > Signed-off-by: Qinglang Miao Reviewed-by: Laurent Pinchart And queued for v5.11. > --- > drivers/gpu/drm/rcar-du/rcar_cmm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drive

Re: [net-next PATCH v2 10/14] device property: Introduce fwnode_get_id()

t > fwnode_handle *fwnode, > > const char *fwnode_get_name(const struct fwnode_handle *fwnode); > const char *fwnode_get_name_prefix(const struct fwnode_handle *fwnode); > +int fwnode_get_id(struct fwnode_handle *fwnode, u32 *id); > struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode); > struct fwnode_handle *fwnode_get_next_parent( > struct fwnode_handle *fwnode); -- Regards, Laurent Pinchart

Re: [PATCH] drivers: usb: gadget: prefer pr_*() functions over raw printk()

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

Re: [PATCH] drivers: usb: gadget: prefer pr_*() functions over raw printk()

; } > > if (int_grp1 & FUSB300_IGR1_DEV_MODE_CHG_INT) { > @@ -1195,11 +1195,11 @@ static irqreturn_t fusb300_irq(int irq, void > *_fusb300) > > if (int_grp1 & FUSB300_IGR1_CX_COMFAIL_INT) { > fusb300_set_cxstall(fusb300); > -

Re: [PATCH 2/4] phy: Add LVDS configuration options

ing > + * the LVDS phy mode. > */ > union phy_configure_opts { > struct phy_configure_opts_mipi_dphy mipi_dphy; > struct phy_configure_opts_dpdp; > + struct phy_configure_opts_lvds lvds; > }; > > /** -- Regards, Laurent Pinchart

Re: [PATCH 06/12] arm64: dts: zynqmp: Add label for zynqmp_ipi

Hi Michal, On Mon, Dec 07, 2020 at 10:39:25AM +0100, Michal Simek wrote: > On 06. 12. 20 23:46, Laurent Pinchart wrote: > > 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. > >>

Re: [PATCH] media: ti-vpe: cal: avoid FIELD_GET assertion

it > __always_inline to avoid the problem. > > Signed-off-by: Arnd Bergmann Reviewed-by: Laurent Pinchart This doesn't conflict with the series I've just sent for the CAL driver, and can thus be applied first or on top. Hans, can I let you handle this ? > --- > drivers/me

Re: [PATCH 06/12] arm64: dts: zynqmp: Add label for zynqmp_ipi

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. > > >

Re: [PATCH 06/12] arm64: dts: zynqmp: Add label for zynqmp_ipi

zynqmp_ipi: zynqmp_ipi { > compatible = "xlnx,zynqmp-ipi-mailbox"; > interrupt-parent = <>; > interrupts = <0 35 4>; -- Regards, Laurent Pinchart

Re: [PATCH 04/12] arm64: dts: zynqmp: Enable and wire reset controller

_RESET_USB0_CORERESET>, > + <_reset ZYNQMP_RESET_USB0_HIBERRESET>, > + <_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 = <_firmware PD_USB_1>; > + resets = <_reset ZYNQMP_RESET_USB1_CORERESET>, > + <_reset ZYNQMP_RESET_USB1_HIBERRESET>, > + <_reset ZYNQMP_RESET_USB1_APB>; > + reset-names = "usb1_crst", "usb1_hibrst", "usb1_apbrst"; > }; > > watchdog0: watchdog@fd4d { -- Regards, Laurent Pinchart

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

Hi Hans, On Tue, Dec 01, 2020 at 09:34:58PM +0100, Hans de Goede wrote: > On 12/1/20 8:21 PM, Andy Shevchenko wrote: > > On Tue, Dec 01, 2020 at 09:06:38PM +0200, Laurent Pinchart wrote: > >> On Tue, Dec 01, 2020 at 09:05:23PM +0200, Andy Shevchenko wrote: > >>> On T

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

On Wed, Dec 02, 2020 at 01:09:56PM +0200, Sakari Ailus wrote: > Hi Laurent, > > On Tue, Dec 01, 2020 at 08:37:58PM +0200, Laurent Pinchart wrote: > > Hi Sakari, > > > > On Tue, Dec 01, 2020 at 05:55:13PM +0200, Sakari Ailus wrote: > > > On Tue, Dec 01, 2020 a

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

both though. > > I think we need those things when we get round to handling the > > VCM/EEPROM that's hidden within the sensor's ACPI entry, but I've not > > done any work on that yet at all. > > Let's consider this later — one step at a time. -- Regards, Laurent Pinchart

Re: [PATCH 13/18] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

Hi Daniel, On Tue, Dec 01, 2020 at 10:08:25PM +, Dan Scally wrote: > On 30/11/2020 17:09, Laurent Pinchart wrote: > > On Mon, Nov 30, 2020 at 01:31:24PM +, Daniel Scally wrote: > >> Currently on platforms designed for Windows, connections between CIO2 and > >&g

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

On Tue, Dec 01, 2020 at 09:05:23PM +0200, Andy Shevchenko wrote: > On Tue, Dec 01, 2020 at 08:55:48PM +0200, Laurent Pinchart wrote: > > On Tue, Dec 01, 2020 at 08:54:17PM +0200, Andy Shevchenko wrote: > > > On Tue, Dec 01, 2020 at 08:30:03AM +, Dan Scally wrote: > >

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

e sense to follow the same approach. That would allow us to avoid large tables of DMI identifiers that would need to be constently updated, potentially making user experience better. -- Regards, Laurent Pinchart

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

Hi Sakari, On Tue, Dec 01, 2020 at 05:55:13PM +0200, Sakari Ailus wrote: > On Tue, Dec 01, 2020 at 01:32:32AM +0200, Laurent Pinchart wrote: > > On Mon, Nov 30, 2020 at 10:07:19PM +0200, Andy Shevchenko wrote: > > > On Mon, Nov 30, 2020 at 01:31:29PM +, D

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

devices, the same HID refers to the camera sensor only. *sigh* :-( > P.S. Dan, can you drop unrelated text when replying? I find a full quote actually useful, as it saves me from having to dig up original e-mails to read missing parts :-) It's a matter of preference I suppose. -- Regards, Laurent Pinchart

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

a CLDB, then the device type would be retrieved from it: > > - If the device is a "discrete PMIC", the driver would register clocks > and regulators controlled by GPIOs, and create clock, regulator and > GPIO lookup entries for the sensor device that references the PMIC. > > - If the device is a TPS64870, the code that exists in the kernel > today to register GPIOs would be activated, and new code would need > to be written to register regulators and clocks. > > - If the device is a uP6641Q, a new driver will need to be written (I > don't know on which devices this PMIC is used, so this can probably > be deferred). > > We can split this in multiple files and/or modules. Could you reply to 20201130233232.gd25...@pendragon.ideasonboard.com to avoid splitting the conversation ? -- Regards, Laurent Pinchart

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

o_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

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

>> +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

Re: [PATCH 02/18] property: Add support for calling fwnode_graph_get_endpoint_by_id() for fwnode->secondary

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

Re: [PATCH 14/18] acpi: utils: Add function to fetch dependent acpi_devices

date == 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

Re: [PATCH 02/18] property: Add support for calling fwnode_graph_get_endpoint_by_id() for fwnode->secondary

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

Re: [PATCH 18/18] ipu3: Add driver for dummy INT3472 ACPI device

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:

Re: [PATCH 15/18] i2c: i2c-core-acpi: Add i2c_acpi_dev_name()

t 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

Re: [PATCH 16/18] i2c: i2c-core-base: Use the new i2c_acpi_dev_name() in i2c_set_dev_name()

{ > - dev_set_name(>dev, "i2c-%s", acpi_dev_name(adev)); > + dev_set_name(>dev, i2c_acpi_dev_name(adev)); > return; > } > -- Regards, Laurent Pinchart

Re: [PATCH 13/18] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

+1702,22 @@ static void cio2_queues_exit(struct cio2_device *cio2) > cio2_queue_exit(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(_dev->dev))) { > + r = cio2_bridge_init(pci_dev); > + if (r) > + return r; > + } > + > r = pcim_enable_device(pci_dev); > if (r) { > dev_err(_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

Re: [PATCH 11/18] media: v4l2-core: v4l2-async: Check possible match in match_fwnode based on sd->fwnode->secondary

atch 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 insertio

Re: [PATCH 07/18] software_node: Add support for fwnode_graph*() family of functions

ode_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

Re: [PATCH 08/18] lib/test_printf.c: Use helper function to unwind array of software_nodes

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.c | 4 +--- >

Re: [PATCH 06/18] software_node: amend software_node_unregister_node_group() to perform unregistration of array in reverse order to be consistent with software_node_unregister_nodes()

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(-) > >

Re: [PATCH 05/18] software_node: Alter software_node_unregister_nodes() to unregister the array in reverse order

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

Re: [PATCH 04/18] software_node: Enforce parent before child ordering of nodes array for software_node_register_nodes()

} > + > ret = software_node_register([i]); > - if (ret) { > - software_node_unregister_nodes(nodes); > - return ret; > - } > + if (ret) > + goto err_unregister_nodes; >

Re: [PATCH 04/18] software_node: Enforce parent before child ordering of nodes array for software_node_register_nodes()

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 unr

Re: [PATCH 03/18] software_node: Fix failure to put() and get() references to children in software_node_get_next_child()

ild - 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

Re: [PATCH 02/18] property: Add support for calling fwnode_graph_get_endpoint_by_id() for fwnode->secondary

Add 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 RF

Re: [PATCH 01/18] property: Return true in fwnode_device_is_available for node types that do not implement this operation

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 RFC v3: >

Re: [PATCH v5 2/3] media: atmel: introduce microchip csi2dc driver

_stream time ? Possibly with a > pm_runtime_get_sync() call ? I see you register pm_runtime operations, > but never call get_sync() or _put(). > > What if CONFIG_PM not selected ? Should this driver select it ? Do you > have use cases where that's not possible ? > > > +} > > + > > +static int csi2dc_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = >dev; > > + struct csi2dc_device *csi2dc; > > + struct resource *res = NULL; > > + int ret = 0; > > + > > + csi2dc = devm_kzalloc(dev, sizeof(*csi2dc), GFP_KERNEL); > > + if (!csi2dc) > > + return -ENOMEM; > > + > > + csi2dc->dev = dev; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > devm_ioremap_resource() checks for the validity of 'res' so you can > drop the previous two lines > > > + > > + csi2dc->base = devm_ioremap_resource(dev, res); > > + > > Is the empy line intentional, sometimes you have one, sometimes you > don't ? > > > + if (IS_ERR(csi2dc->base)) { > > + dev_err(dev, "base address not set\n"); > > > > + return PTR_ERR(csi2dc->base); > > + } > > + > > + csi2dc->pclk = devm_clk_get(dev, "pclk"); > > + if (IS_ERR(csi2dc->pclk)) { > > + ret = PTR_ERR(csi2dc->pclk); > > + dev_err(dev, "failed to get pclk: %d\n", ret); > > + return ret; > > Just return PTR_ERR instead of assigning to ret > > > + } > > + > > + ret = clk_prepare_enable(csi2dc->pclk); > > + if (ret) { > > + dev_err(dev, "failed to enable pclk: %d\n", ret); > > + return ret; > > + } > > Has this clock to be enabled here or can it be done in the > pm_runtime callback ? > > > + > > + csi2dc->scck = devm_clk_get(dev, "scck"); > > + if (IS_ERR(csi2dc->scck)) { > > + ret = PTR_ERR(csi2dc->scck); > > + dev_err(dev, "failed to get scck: %d\n", ret); > > + goto csi2dc_clk_fail; > > + } > > + > > + ret = v4l2_device_register(dev, >v4l2_dev); > > + if (ret) { > > + dev_err(dev, "unable to register v4l2 device.\n"); > > + goto csi2dc_clk_fail; > > + } > > + > > + v4l2_subdev_init(>csi2dc_sd, _subdev_ops); > > + > > + csi2dc->csi2dc_sd.owner = THIS_MODULE; > > + csi2dc->csi2dc_sd.dev = dev; > > + snprintf(csi2dc->csi2dc_sd.name, sizeof(csi2dc->csi2dc_sd.name), > > +"CSI2DC.0"); > > + > > + csi2dc->csi2dc_sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > > + csi2dc->csi2dc_sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE; > > + csi2dc->pads[CSI2DC_PAD_SINK].flags = MEDIA_PAD_FL_SINK; > > + csi2dc->pads[CSI2DC_PAD_SOURCE].flags = MEDIA_PAD_FL_SOURCE; > > Isn't the source pad presence conditional to the presence of > port@1 ? > > > + > > + ret = media_entity_pads_init(>csi2dc_sd.entity, CSI2DC_PADS_NUM, > > +csi2dc->pads); > > + if (ret < 0) { > > + dev_err(dev, "media entity init failed\n"); > > + goto csi2dc_probe_entity_err; > > + } > > + > > + v4l2_set_subdevdata(>csi2dc_sd, pdev); > > + > > + platform_set_drvdata(pdev, >csi2dc_sd); > > + > > + INIT_WORK(>workq, csi2dc_workq_handler); > > + > > + ret = csi2dc_of_parse(csi2dc, dev->of_node); > > + if (ret) > > + goto csi2dc_probe_entity_err; > > + > > + dev_info(dev, "Microchip CSI2DC version %x\n", > > +csi2dc_readl(csi2dc, CSI2DC_VERSION)); > > + > > + pm_runtime_set_active(dev); > > + pm_runtime_enable(dev); > > + pm_request_idle(dev); > > + > > + return 0; > > + > > +csi2dc_probe_entity_err: > > + media_entity_cleanup(>csi2dc_sd.entity); > > + v4l2_device_unregister(>v4l2_dev); > > +csi2dc_clk_fail: > > + clk_disable_unprepare(csi2dc->pclk); > > + return ret; > > +} > > + > > +static int csi2dc_remove(struct platform_device *pdev) > > +{ > > + struct v4l2_subdev *csi2dc_sd = platform_get_drvdata(pdev); > > + struct csi2dc_device *csi2dc = csi2dc_sd_to_csi2dc_device(csi2dc_sd); > > + > > + pm_runtime_disable(>dev); > > + > > + v4l2_async_unregister_subdev(>csi2dc_sd); > > + csi2dc_cleanup_notifier(csi2dc); > > + media_entity_cleanup(>csi2dc_sd.entity); > > + v4l2_device_unregister(>v4l2_dev); > > + clk_disable_unprepare(csi2dc->pclk); > > + > > + return 0; > > +} > > + > > +static int __maybe_unused csi2dc_runtime_suspend(struct device *dev) > > +{ > > + struct csi2dc_device *csi2dc = dev_get_drvdata(dev); > > + > > + return csi2dc_power(csi2dc, false); > > +} > > + > > +static int __maybe_unused csi2dc_runtime_resume(struct device *dev) > > +{ > > + struct csi2dc_device *csi2dc = dev_get_drvdata(dev); > > + > > + return csi2dc_power(csi2dc, true); > > +} > > + > > +static const struct dev_pm_ops csi2dc_dev_pm_ops = { > > + SET_RUNTIME_PM_OPS(csi2dc_runtime_suspend, csi2dc_runtime_resume, NULL) > > +}; > > + > > +static const struct of_device_id csi2dc_of_match[] = { > > + { .compatible = "microchip,sama7g5-csi2dc" }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(of, csi2dc_of_match); > > + > > +static struct platform_driver csi2dc_driver = { > > + .probe = csi2dc_probe, > > + .remove = csi2dc_remove, > > + .driver = { > > + .name = "microchip-csi2dc", > > + .pm = _dev_pm_ops, > > + .of_match_table = of_match_ptr(csi2dc_of_match), > > + }, > > +}; > > + > > +module_platform_driver(csi2dc_driver); > > + > > +MODULE_AUTHOR("Eugen Hristev "); > > +MODULE_DESCRIPTION("Microchip CSI2 Demux Controller driver"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_SUPPORTED_DEVICE("video"); > > For my education, what's MODULE_SUPPORTED_DEVICE for ? It's "not yet implemented" since 2005, I think it can be dropped :-) -- Regards, Laurent Pinchart

Re: [RFC PATCH v3 9/9] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

Hi Andy, On Mon, Nov 16, 2020 at 03:57:17PM +0200, Andy Shevchenko wrote: > On Mon, Nov 16, 2020 at 10:53 AM Laurent Pinchart wrote: > > On Fri, Nov 13, 2020 at 09:45:00PM +0200, Andy Shevchenko wrote: > > > On Fri, Nov 13, 2020 at 6:22 PM Laurent Pinchart wrote: > >

Re: [PATCH v4 1/8] media: i2c: Add driver for RDACM21 camera module

; > > + v4l2_ctrl_new_std(>ctrls, NULL, V4L2_CID_PIXEL_RATE, > > > + OV10640_PIXEL_RATE, OV10640_PIXEL_RATE, 1, > > > + OV10640_PIXEL_RATE); > > > + dev->sd.ctrl_handler = >ctrls; > > > + > > > + ret = dev->ctrls.error; > &g

Re: [RFC PATCH v3 9/9] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

Hi Andy, On Fri, Nov 13, 2020 at 09:45:00PM +0200, Andy Shevchenko wrote: > On Fri, Nov 13, 2020 at 6:22 PM Laurent Pinchart wrote: > > On Fri, Nov 13, 2020 at 10:02:30AM +, Dan Scally wrote: > > > On 29/10/2020 22:51, Laurent Pinchart wrote: > > > > On Fri, Oc

Re: [RFC PATCH v3 9/9] ipu3-cio2: Add functionality allowing software_node connections to sensors on platforms designed for Windows

Hi Dan, On Fri, Nov 13, 2020 at 10:02:30AM +, Dan Scally wrote: > On 29/10/2020 22:51, Laurent Pinchart wrote: > > On Fri, Oct 30, 2020 at 12:22:15AM +0200, Andy Shevchenko wrote: > >> On Thu, Oct 29, 2020 at 11:29:30PM +0200, Laurent Pinchart wrote: > >>> On T

Re: [PATCH v3 1/3] dt-bindings: Convert graph bindings to json-schema

Hi Rob, On Wed, Nov 11, 2020 at 05:03:26PM -0600, Rob Herring wrote: > On Wed, Nov 11, 2020 at 8:27 AM Laurent Pinchart wrote: > > On Wed, Nov 11, 2020 at 08:25:40AM -0600, Rob Herring wrote: > > > On Wed, Nov 11, 2020 at 8:00 AM Laurent Pinchart wrote: > > > > On M

Re: [PATCH v3 1/3] dt-bindings: Convert graph bindings to json-schema

Hi Rob, On Wed, Nov 11, 2020 at 08:25:40AM -0600, Rob Herring wrote: > On Wed, Nov 11, 2020 at 8:00 AM Laurent Pinchart wrote: > > On Mon, Nov 02, 2020 at 02:36:54PM -0600, Rob Herring wrote: > > > From: Sameer Pujar > > > > > > Convert device tree binding

Re: [PATCH v3 3/3] dt-bindings: panel: common: Add reference to graph schema

Hi Rob, Thank you for the patch. On Mon, Nov 02, 2020 at 02:36:56PM -0600, Rob Herring wrote: > Now that we have a graph schema, reference it from the common panel > schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Laurent Pinchart > Signed-off-by: Rob Herring

<    1   2   3   4   5   6   7   8   9   10   >