Re: [PATCH v2] videodev2.h: add helper to validate colorspace
On 02/21/2018 09:16 PM, Laurent Pinchart wrote: > Hi Hans, > > On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote: >> On 02/19/2018 11:28 PM, Niklas Söderlund wrote: >>> Hi Hans, >>> >>> Thanks for your feedback. >>> >>> [snip] >>> Can you then fix v4l2-compliance to stop testing colorspace against 0xff ? >>> >>> For now I can simply relax this test for subdevs with sources and >>> sinks. >> >> You also need to relax it for video nodes with MC drivers, as the DMA >> engines don't care about colorspaces. > > Yes, they do. Many DMA engines can at least do RGB <-> YUV conversions, > so they should get the colorspace info from their source and pass it on > to userspace (after correcting for any conversions done by the DMA > engine). Not in the MC case. Video nodes there only model the DMA engine, and are thus not aware of colorspaces. What MC drivers do is check at stream on time when validating the pipeline that the colorspace set by userspace on the video node corresponds to the colorspace on the source pad of the connected subdev, but that's only to ensure that userspace gets a coherent view of colorspace across the pipeline, not to program the hardware. There could be exceptions, but in the general case, the video node implementation of an MC driver will accept any colorspace and only validate it at stream on time, similarly to how it does for the frame size format instance (and in the frame size case it will usually enforce min/max limits when the DMA engine limits the frame size).> >>> I'm afraid the issue described above by Laurent is what sparked me to >>> write this commit to begin with. In my never ending VIN Gen3 patch-set I >>> currency need to carry a patch [1] to implement a hack to make sure >>> v4l2-compliance do not fail for the VIN Gen3 MC-centric use-case. This >>> patch was an attempt to be able to validate the colorspace using the >>> magic value 0xff. >> >> This is NOT a magic value. The test that's done here is to memset the >> format structure with 0xff, then call the ioctl. Afterwards it checks >> if there are any remaining 0xff bytes left in the struct since it expects >> the driver to have overwritten it by something else. That's where the 0xff >> comes from. > > It's no less or more magic than using 0xdeadbeef or any fixed value :-) I > think we all agree that it isn't a value that is meant to be handled > specifically by drivers, so it's not magic in that sense. > >>> I don't feel strongly for this patch in particular and I'm happy to drop >>> it. But I would like to receive some guidance on how to then properly >>> be able to handle this problem for the MC-centric VIN driver use-case. >>> One option is as you suggested to relax the test in v4l-compliance to >>> not check colorspace, but commit [2] is not enough to resolve the issue >>> for my MC use-case. >>> >>> As Laurent stated above, the use-case is that the video device shall >>> accept any colorspace set from user-space. This colorspace is then only >>> used as stream on time to validate the MC pipeline. The VIN driver do >>> not care about colorspace, but I care about not breaking v4l2-compliance >>> as I find it's a very useful tool :-) >> >> I think part of my confusion here is that there are two places where you >> deal with colorspaces in a DMA engine: first there is a input pad of the >> DMA engine entity, secondly there is the v4l2_pix_format for the memory >> description. >> >> The second is set by the driver based on what userspace specified for the >> input pad, together with any changes due to additional conversions such >> as quantization range and RGB <-> YUV by the DMA engine. > > No, I'm sorry, for MC-based drivers this isn't correct. The media entity that > symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a > subdev. It thus has no media bus format configured for its sink pad. The > closest pad in the pipeline that has a media bus format is the source pad of > the subdev connected to the video node. > > There's no communication within the kernel at G/S_FMT time between the video > node and its connected subdev. The only time we look at the pipeline as a > whole is when starting the stream to validate that the pipeline is correctly > configured. We thus have to implement G/S_FMT on the video node without any > knowledge about the connected subdev, and thus accept any colorspace. > >> So any colorspace validation is done for the input pad. The question is >> what that validation should be. It's never been defined. > > No format is set on the video node's entity sink pad for the reason above, so > no validation occurs when setting the colorspace on the sink pad as that > never > happens. Is this documented anywhere? Certainly VIDIOC_G/S/TRY_FMT doesn't mention it. It is certainly a surprise to me. Regards, Hans
Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
On 02/20/18 15:10, Laurent Pinchart wrote: > Hello, > > This patch series addresses a design mistake that dates back from the initial > DU support. Support for the LVDS encoders, which are IP cores separate from > the DU, was bundled in the DU driver. Worse, both the DU and LVDS were > described through a single DT node. > > To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS > encoders, and deprecate their description inside the DU bindings. To retain > backward compatibility with existing DT, patches 03/16 to 08/16 then patch the > device tree at runtime to convert the legacy bindings to the new ones. > > With the DT side addressed, patch 09/16 converts the LVDS support code to a > separate bridge driver. Patches 11/16 to 16/16 then update all the device tree > sources to the new DU and LVDS encoders bindings. > > I decided to go for live DT patching in patch 08/16 because implementing > support for both the legacy and new bindings in the driver would have been > very intrusive, and prevented further cleanups. This version relies more > heavily on overlays to avoid touching the internals of the OF core compared to > v2, even if manual fixes to the device tree are still needed. > > Compared to v3, this series uses the OF changeset API to update properties > instead of accessing the internals of the property structure. This removes the > local implementation of functions to look up nodes by path and update > properties. In order to do this, I pulled in Pantelis' patch series titled > "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and > rebased it while taking two small review comments into account. Wait a minute! Why are you putting a patch set to modify core devicetree in the middle of a driver series. Please pull it out to a separate series. I'll try to look at the patches, as they are in this series, sometime tomorrow. I have a vague memory of unresolved issues from the last time they were proposed. Thanks, Frank > > Rob, I'd like this series to be merged in v4.17. As the changeset helpers are > now a dependency, I'd need you to merge them early (ideally on top of > v4.16-rc1) and provide a stable branch, or get your ack to merge them through > Dave's tree if they don't conflict with what you have and will queue for > v4.17. > > This version also drops the small fix to the Porter board device tree that has > been queued for v4.17 already. > > Compared to v2, the biggest change is in patch 03/16. Following Rob's and > Frank's reviews it was clear that modifying the unflattened DT structure of > the overlay before applying it wasn't popular. I have thus decided to use one > overlay source per SoC to move as much of the DT changes to the overlay as > possible, and only perform manual modifications (that are still needed as some > of the information is board-specific) on the system DT after applying the > overlay. As a result the overlay is parsed and applied without being modified. > > Compared to v1, this series update the r8a7792 and r8a7794 device tree sources > and incorporate review feedback as described by the changelogs of individual > patches. > > > Laurent Pinchart (11): > dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings > dt-bindings: display: renesas: Deprecate LVDS support in the DU > bindings > drm: rcar-du: Fix legacy DT to create LVDS encoder nodes > drm: rcar-du: Convert LVDS encoder code to bridge driver > ARM: dts: r8a7790: Convert to new LVDS DT bindings > ARM: dts: r8a7791: Convert to new LVDS DT bindings > ARM: dts: r8a7792: Convert to new DU DT bindings > ARM: dts: r8a7793: Convert to new LVDS DT bindings > ARM: dts: r8a7794: Convert to new DU DT bindings > arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings > arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings > > Pantelis Antoniou (5): > of: dynamic: Add __of_node_dupv() > of: changesets: Introduce changeset helper methods > of: changeset: Add of_changeset_node_move method > of: unittest: changeset helpers > i2c: demux: Use changeset helpers for clarity > > .../bindings/display/bridge/renesas,lvds.txt | 56 +++ > .../devicetree/bindings/display/renesas,du.txt | 31 +- > MAINTAINERS| 1 + > arch/arm/boot/dts/r8a7790-lager.dts| 22 +- > arch/arm/boot/dts/r8a7790.dtsi | 64 ++- > arch/arm/boot/dts/r8a7791-koelsch.dts | 10 +- > arch/arm/boot/dts/r8a7791-porter.dts | 16 +- > arch/arm/boot/dts/r8a7791.dtsi | 36 +- > arch/arm/boot/dts/r8a7792.dtsi | 1 - > arch/arm/boot/dts/r8a7793-gose.dts | 10 +- > arch/arm/boot/dts/r8a7793.dtsi | 37 +- > arch/arm/boot/dts/r8a7794.dtsi | 1 - > .../boot/dts/renesas/r8a7795-es1-salvator-x.dts| 3 +- > arch/arm64/boot/dts/renesas/r8a7
[PATCH v5 0/8] R-Car DU: Convert LVDS code to bridge driver
Hello, This patch series addresses a design mistake that dates back from the initial DU support. Support for the LVDS encoders, which are IP cores separate from the DU, was bundled in the DU driver. Worse, both the DU and LVDS were described through a single DT node. To fix the, patches 1/8 and 2/8 define new DT bindings for the LVDS encoders, and deprecate their description inside the DU bindings. To retain backward compatibility with existing DT, patches 3/8 to 7/8 then patch the device tree at runtime to convert the legacy bindings to the new ones. With the DT side addressed, patch 8/8 converts the LVDS support code to a separate bridge driver. I decided to go for live DT patching in patch 7/8 because implementing support for both the legacy and new bindings in the driver would have been very intrusive, and prevented further cleanups. This version relies more heavily on overlays to avoid touching the internals of the OF core compared to v2, even if manual fixes to the device tree are still needed. As all the patches but the last one have been acked, I plan to send a pull request by the end of the week if no new issue is discovered. Compared to v4, the patch "of: changeset: Add of_changeset_node_move method" has been dropped as it's not needed. The patches that update the DT sources to the new DU and LVDS bindings have been dropped as well to avoid spamming the lists as they depend on the driver changes going in first to avoid regressions and haven't been changed. Compared to v3, this series uses the OF changeset API to update properties instead of accessing the internals of the property structure. This removes the local implementation of functions to look up nodes by path and update properties. In order to do this, I pulled in Pantelis' patch series titled "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and rebased it while taking two small review comments into account. Rob, I'd like this series to be merged in v4.17. As the changeset helpers are now a dependency, I'd need you to merge them early (ideally on top of v4.16-rc1) and provide a stable branch, or get your ack to merge them through Dave's tree if they don't conflict with what you have and will queue for v4.17. This version also drops the small fix to the Porter board device tree that has been queued for v4.17 already. Compared to v2, the biggest change is in patch 7/8. Following Rob's and Frank's reviews it was clear that modifying the unflattened DT structure of the overlay before applying it wasn't popular. I have thus decided to use one overlay source per SoC to move as much of the DT changes to the overlay as possible, and only perform manual modifications (that are still needed as some of the information is board-specific) on the system DT after applying the overlay. As a result the overlay is parsed and applied without being modified. Compared to v1, this series update the r8a7792 and r8a7794 device tree sources and incorporate review feedback as described by the changelogs of individual patches. Laurent Pinchart (4): dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings drm: rcar-du: Fix legacy DT to create LVDS encoder nodes drm: rcar-du: Convert LVDS encoder code to bridge driver Pantelis Antoniou (4): of: dynamic: Add __of_node_dupv() of: changesets: Introduce changeset helper methods of: unittest: changeset helpers i2c: demux: Use changeset helpers for clarity .../bindings/display/bridge/renesas,lvds.txt | 56 +++ .../devicetree/bindings/display/renesas,du.txt | 31 +- MAINTAINERS| 1 + drivers/gpu/drm/rcar-du/Kconfig| 6 +- drivers/gpu/drm/rcar-du/Makefile | 10 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 - drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +-- drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 12 - drivers/gpu/drm/rcar-du/rcar_du_kms.c | 14 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 93 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 24 - drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 -- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 64 --- drivers/gpu/drm/rcar-du/rcar_du_of.c | 307 drivers/gpu/drm/rcar-du/rcar_du_of.h | 20 + .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts| 79 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts| 53 +++ .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts| 53 +++ .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts| 53 +++ .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts| 53 +++ drivers/gpu/drm/rcar-du/rcar_lvds.c| 524 + drivers/i2c/muxes/i2c-demux-pinctrl.c | 12 +- driver
[PATCH v5 3/8] of: dynamic: Add __of_node_dupv()
From: Pantelis Antoniou Add an __of_node_dupv() private method and make __of_node_dup() use it. This is required for the subsequent changeset accessors which will make use of it. Signed-off-by: Pantelis Antoniou [Make __of_node_dupv() static] Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- drivers/of/dynamic.c | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 7bb33d22b4e2..a2f0c45836f9 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags) } /** - * __of_node_dup() - Duplicate or create an empty device node dynamically. - * @fmt: Format string (plus vargs) for new full name of the device node + * __of_node_dupv() - Duplicate or create an empty device node dynamically. + * @fmt: Format string for new full name of the device node + * @vargs: va_list containing the arugments for the node full name * * Create an device tree node, either by duplicating an empty node or by allocating * an empty one suitable for further modification. The node data are @@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags) * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of * memory error. */ -struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...) +static struct device_node *__of_node_dupv(const struct device_node *np, + const char *fmt, va_list vargs) { - va_list vargs; struct device_node *node; node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) return NULL; - va_start(vargs, fmt); node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs); - va_end(vargs); if (!node->full_name) { kfree(node); return NULL; @@ -433,6 +432,24 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, return NULL; } +/** + * __of_node_dup() - Duplicate or create an empty device node dynamically. + * @fmt: Format string (plus vargs) for new full name of the device node + * + * See: __of_node_dupv() + */ +struct device_node *__of_node_dup(const struct device_node *np, + const char *fmt, ...) +{ + va_list vargs; + struct device_node *node; + + va_start(vargs, fmt); + node = __of_node_dupv(np, fmt, vargs); + va_end(vargs); + return node; +} + static void __of_changeset_entry_destroy(struct of_changeset_entry *ce) { of_node_put(ce->np); -- Regards, Laurent Pinchart
[PATCH v5 2/8] dt-bindings: display: renesas: Deprecate LVDS support in the DU bindings
The internal LVDS encoders now have their own DT bindings, representing them as part of the DU is deprecated. Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- Changes since v1: - Remove the LVDS reg range from the example - Remove the reg-names property --- .../devicetree/bindings/display/renesas,du.txt | 31 -- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt index cd48aba3bc8c..e79cf9b0ad38 100644 --- a/Documentation/devicetree/bindings/display/renesas,du.txt +++ b/Documentation/devicetree/bindings/display/renesas,du.txt @@ -14,12 +14,7 @@ Required Properties: - "renesas,du-r8a7795" for R8A7795 (R-Car H3) compatible DU - "renesas,du-r8a7796" for R8A7796 (R-Car M3-W) compatible DU - - reg: A list of base address and length of each memory resource, one for -each entry in the reg-names property. - - reg-names: Name of the memory resources. The DU requires one memory -resource for the DU core (named "du") and one memory resource for each -LVDS encoder (named "lvds.x" with "x" being the LVDS controller numerical -index). + - reg: the memory-mapped I/O registers base address and length - interrupt-parent: phandle of the parent interrupt controller. - interrupts: Interrupt specifiers for the DU interrupts. @@ -29,14 +24,13 @@ Required Properties: - clock-names: Name of the clocks. This property is model-dependent. - R8A7779 uses a single functional clock. The clock doesn't need to be named. -- All other DU instances use one functional clock per channel and one - clock per LVDS encoder (if available). The functional clocks must be - named "du.x" with "x" being the channel numerical index. The LVDS clocks - must be named "lvds.x" with "x" being the LVDS encoder numerical index. -- In addition to the functional and encoder clocks, all DU versions also - support externally supplied pixel clocks. Those clocks are optional. - When supplied they must be named "dclkin.x" with "x" being the input - clock numerical index. +- All other DU instances use one functional clock per channel The + functional clocks must be named "du.x" with "x" being the channel + numerical index. +- In addition to the functional clocks, all DU versions also support + externally supplied pixel clocks. Those clocks are optional. When + supplied they must be named "dclkin.x" with "x" being the input clock + numerical index. - vsps: A list of phandle and channel index tuples to the VSPs that handle the memory interfaces for the DU channels. The phandle identifies the VSP @@ -69,9 +63,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU du: display@feb0 { compatible = "renesas,du-r8a7795"; - reg = <0 0xfeb0 0 0x8>, - <0 0xfeb9 0 0x14>; - reg-names = "du", "lvds.0"; + reg = <0 0xfeb0 0 0x8>; interrupts = , , , @@ -79,9 +71,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU clocks = <&cpg CPG_MOD 724>, <&cpg CPG_MOD 723>, <&cpg CPG_MOD 722>, -<&cpg CPG_MOD 721>, -<&cpg CPG_MOD 727>; - clock-names = "du.0", "du.1", "du.2", "du.3", "lvds.0"; +<&cpg CPG_MOD 721>; + clock-names = "du.0", "du.1", "du.2", "du.3"; vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>; ports { -- Regards, Laurent Pinchart
[PATCH v5 4/8] of: changesets: Introduce changeset helper methods
From: Pantelis Antoniou Changesets are very powerful, but the lack of a helper API makes using them cumbersome. Introduce a simple copy based API that makes things considerably easier. To wit, adding a property using the raw API. struct property *prop; prop = kzalloc(sizeof(*prop)), GFP_KERNEL); prop->name = kstrdup("compatible"); prop->value = kstrdup("foo,bar"); prop->length = strlen(prop->value) + 1; of_changeset_add_property(ocs, np, prop); while using the helper API of_changeset_add_property_string(ocs, np, "compatible", "foo,bar"); Signed-off-by: Pantelis Antoniou [Fixed memory leak in __of_changeset_add_update_property_copy()] [Fixed cpu to be32 conversion sparse warnings] [Move include to fix compilation when !CONFIG_OF_DYNAMIC] Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- drivers/of/dynamic.c | 222 ++ include/linux/of.h | 329 +++ 2 files changed, 551 insertions(+) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index a2f0c45836f9..f62083921df2 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -910,3 +910,225 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action, return 0; } EXPORT_SYMBOL_GPL(of_changeset_action); + +/* changeset helpers */ + +/** + * of_changeset_create_device_node - Create an empty device node + * + * @ocs: changeset pointer + * @parent:parent device node + * @fmt: format string for the node's full_name + * @args: argument list for the format string + * + * Create an empty device node, marking it as detached and allocated. + * + * Returns a device node on success, an error encoded pointer otherwise + */ +struct device_node *of_changeset_create_device_nodev( + struct of_changeset *ocs, struct device_node *parent, + const char *fmt, va_list vargs) +{ + struct device_node *node; + + node = __of_node_dupv(NULL, fmt, vargs); + if (!node) + return ERR_PTR(-ENOMEM); + + node->parent = parent; + return node; +} +EXPORT_SYMBOL_GPL(of_changeset_create_device_nodev); + +/** + * of_changeset_create_device_node - Create an empty device node + * + * @ocs: changeset pointer + * @parent:parent device node + * @fmt: Format string for the node's full_name + * ... Arguments + * + * Create an empty device node, marking it as detached and allocated. + * + * Returns a device node on success, an error encoded pointer otherwise + */ +__printf(3, 4) struct device_node * +of_changeset_create_device_node(struct of_changeset *ocs, + struct device_node *parent, const char *fmt, ...) +{ + va_list vargs; + struct device_node *node; + + va_start(vargs, fmt); + node = of_changeset_create_device_nodev(ocs, parent, fmt, vargs); + va_end(vargs); + return node; +} +EXPORT_SYMBOL_GPL(of_changeset_create_device_node); + +/** + * __of_changeset_add_property_copy - Create/update a new property copying + *name & value + * + * @ocs: changeset pointer + * @np:device node pointer + * @name: name of the property + * @value: pointer to the value data + * @length:length of the value in bytes + * @update:True on update operation + * + * Adds/updates a property to the changeset by making copies of the name & value + * entries. The @update parameter controls whether an add or update takes place. + * + * Returns zero on success, a negative error value otherwise. + */ +int __of_changeset_add_update_property_copy(struct of_changeset *ocs, + struct device_node *np, const char *name, const void *value, + int length, bool update) +{ + struct property *prop; + int ret = -ENOMEM; + + prop = kzalloc(sizeof(*prop), GFP_KERNEL); + if (!prop) + return -ENOMEM; + + prop->name = kstrdup(name, GFP_KERNEL); + if (!prop->name) + goto out_err; + + /* +* NOTE: There is no check for zero length value. +* In case of a boolean property, this will allocate a value +* of zero bytes. We do this to work around the use +* of of_get_property() calls on boolean values. +*/ + prop->value = kmemdup(value, length, GFP_KERNEL); + if (!prop->value) + goto out_err; + + of_property_set_flag(prop, OF_DYNAMIC); + + prop->length = length; + + if (!update) + ret = of_changeset_add_property(ocs, np, prop); + else + ret = of_changeset_update_property(ocs, np, prop); + + if (!ret) + return 0; + +out_err: + kfree(prop->value); + kfree(prop->name); + kfree(prop); + return ret; +} +EXPORT_SYMBOL_GPL(__of_changeset_add_update_property_copy); + +/** + * of_changes
[PATCH v5 6/8] i2c: demux: Use changeset helpers for clarity
From: Pantelis Antoniou The changeset helpers are easier to use, use them instead of using the static property. Signed-off-by: Pantelis Antoniou Acked-by: Wolfram Sang ["okay" -> "ok"] Signed-off-by: Laurent Pinchart --- drivers/i2c/muxes/i2c-demux-pinctrl.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c index 33ce032cb701..0f0046831492 100644 --- a/drivers/i2c/muxes/i2c-demux-pinctrl.c +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c @@ -220,10 +220,7 @@ static int i2c_demux_pinctrl_probe(struct platform_device *pdev) priv = devm_kzalloc(&pdev->dev, sizeof(*priv) + num_chan * sizeof(struct i2c_demux_pinctrl_chan), GFP_KERNEL); - - props = devm_kcalloc(&pdev->dev, num_chan, sizeof(*props), GFP_KERNEL); - - if (!priv || !props) + if (!priv) return -ENOMEM; err = of_property_read_string(np, "i2c-bus-name", &priv->bus_name); @@ -241,12 +238,9 @@ static int i2c_demux_pinctrl_probe(struct platform_device *pdev) } priv->chan[i].parent_np = adap_np; - props[i].name = devm_kstrdup(&pdev->dev, "status", GFP_KERNEL); - props[i].value = devm_kstrdup(&pdev->dev, "ok", GFP_KERNEL); - props[i].length = 3; - of_changeset_init(&priv->chan[i].chgset); - of_changeset_update_property(&priv->chan[i].chgset, adap_np, &props[i]); + of_changeset_update_property_string(&priv->chan[i].chgset, + adap_np, "status", "ok"); } priv->num_chan = num_chan; -- Regards, Laurent Pinchart
[PATCH v5 8/8] drm: rcar-du: Convert LVDS encoder code to bridge driver
The LVDS encoders used to be described in DT as part of the DU. They now have their own DT node, linked to the DU using the OF graph bindings. This allows moving internal LVDS encoder support to a separate driver modelled as a DRM bridge. Backward compatibility is retained as legacy DT is patched live to move to the new bindings. Signed-off-by: Laurent Pinchart --- Changes since v1: - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only - Update to the -lvds compatible string format --- drivers/gpu/drm/rcar-du/Kconfig | 4 +- drivers/gpu/drm/rcar-du/Makefile | 3 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 21 +- drivers/gpu/drm/rcar-du/rcar_du_drv.h | 5 - drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 175 +- drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 12 - drivers/gpu/drm/rcar-du/rcar_du_kms.c | 14 +- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 93 -- drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h | 24 -- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c | 238 -- drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h | 64 drivers/gpu/drm/rcar-du/rcar_lvds.c | 524 ++ 12 files changed, 561 insertions(+), 616 deletions(-) delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdscon.h delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.c delete mode 100644 drivers/gpu/drm/rcar-du/rcar_du_lvdsenc.h create mode 100644 drivers/gpu/drm/rcar-du/rcar_lvds.c diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig index 3f83352a7313..edde8d4b87a3 100644 --- a/drivers/gpu/drm/rcar-du/Kconfig +++ b/drivers/gpu/drm/rcar-du/Kconfig @@ -19,8 +19,8 @@ config DRM_RCAR_DW_HDMI Enable support for R-Car Gen3 internal HDMI encoder. config DRM_RCAR_LVDS - bool "R-Car DU LVDS Encoder Support" - depends on DRM_RCAR_DU + tristate "R-Car DU LVDS Encoder Support" + depends on DRM && DRM_BRIDGE && OF select DRM_PANEL select OF_FLATTREE select OF_OVERLAY diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile index 86b337b4be5d..3e58ed93d5b1 100644 --- a/drivers/gpu/drm/rcar-du/Makefile +++ b/drivers/gpu/drm/rcar-du/Makefile @@ -4,10 +4,8 @@ rcar-du-drm-y := rcar_du_crtc.o \ rcar_du_encoder.o \ rcar_du_group.o \ rcar_du_kms.o \ -rcar_du_lvdscon.o \ rcar_du_plane.o -rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \ rcar_du_of_lvds_r8a7790.dtb.o \ rcar_du_of_lvds_r8a7791.dtb.o \ @@ -18,3 +16,4 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)+= rcar_du_vsp.o obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o +obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 6e02c762a557..06a3fbdd728a 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -29,6 +29,7 @@ #include "rcar_du_drv.h" #include "rcar_du_kms.h" +#include "rcar_du_of.h" #include "rcar_du_regs.h" /* - @@ -74,7 +75,6 @@ static const struct rcar_du_device_info rzg1_du_r8a7745_info = { .port = 1, }, }, - .num_lvds = 0, }; static const struct rcar_du_device_info rcar_du_r8a7779_info = { @@ -95,14 +95,13 @@ static const struct rcar_du_device_info rcar_du_r8a7779_info = { .port = 1, }, }, - .num_lvds = 0, }; static const struct rcar_du_device_info rcar_du_r8a7790_info = { .gen = 2, .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK | RCAR_DU_FEATURE_EXT_CTRL_REGS, - .quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES, + .quirks = RCAR_DU_QUIRK_ALIGN_128B, .num_crtcs = 3, .routes = { /* @@ -164,7 +163,6 @@ static const struct rcar_du_device_info rcar_du_r8a7792_info = { .port = 1, }, }, - .num_lvds = 0, }; static const struct rcar_du_device_info rcar_du_r8a7794_info = { @@ -186,7 +184,6 @@ static const struct rcar_du_device_info rcar_du_r8a7794_info = { .port = 1, }, }, - .num_lvds = 0, }; static const struct rcar_du_device_info rcar_du_r8a7795_info = { @@ -434,7 +431,19 @@ static struct platform_driver rcar_du_platform_driver = { }, }; -module_platform_driver(rcar_du_platform_driver); +static int __init rcar_du_init(void) +{ + rcar_du_of_init(rcar_du_of_table); + +
[PATCH v5 5/8] of: unittest: changeset helpers
From: Pantelis Antoniou Add a unitest specific for the new changeset helpers. Signed-off-by: Pantelis Antoniou [Use IS_ENABLED instead of #ifdef] Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- drivers/of/unittest.c | 54 +++ 1 file changed, 54 insertions(+) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 7a9abaae874d..1b21d2c549a8 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void) #endif } +static void __init of_unittest_changeset_helper(void) +{ +#ifdef CONFIG_OF_DYNAMIC + struct device_node *n1, *n2, *n21, *parent, *np; + struct of_changeset chgset; + + of_changeset_init(&chgset); + + parent = of_find_node_by_path("/testcase-data/changeset"); + + unittest(parent, "testcase setup failure\n"); + n1 = of_changeset_create_device_node(&chgset, + parent, "/testcase-data/changeset/n1"); + unittest(n1, "testcase setup failure\n"); + n2 = of_changeset_create_device_node(&chgset, + parent, "/testcase-data/changeset/n2"); + unittest(n2, "testcase setup failure\n"); + n21 = of_changeset_create_device_node(&chgset, n2, "%s/%s", + "/testcase-data/changeset/n2", "n21"); + unittest(n21, "testcase setup failure\n"); + + unittest(!of_changeset_add_property_string(&chgset, parent, + "prop-add", "foo"), "fail add prop\n"); + + unittest(!of_changeset_attach_node(&chgset, n1), "fail n1 attach\n"); + unittest(!of_changeset_attach_node(&chgset, n2), "fail n2 attach\n"); + unittest(!of_changeset_attach_node(&chgset, n21), "fail n21 attach\n"); + + unittest(!of_changeset_apply(&chgset), "apply failed\n"); + + /* Make sure node names are constructed correctly */ + np = of_find_node_by_path("/testcase-data/changeset/n1"); + unittest(np, "'%s' not added\n", n1->full_name); + of_node_put(np); + + /* Make sure node names are constructed correctly */ + np = of_find_node_by_path("/testcase-data/changeset/n2"); + unittest(np, "'%s' not added\n", n2->full_name); + of_node_put(np); + + np = of_find_node_by_path("/testcase-data/changeset/n2/n21"); + unittest(np, "'%s' not added\n", n21->full_name); + of_node_put(np); + + unittest(!of_changeset_revert(&chgset), "revert failed\n"); + + of_changeset_destroy(&chgset); + + of_node_put(parent); +#endif +} + + static void __init of_unittest_parse_interrupts(void) { struct device_node *np; @@ -2363,6 +2416,7 @@ static int __init of_unittest(void) of_unittest_property_string(); of_unittest_property_copy(); of_unittest_changeset(); + of_unittest_changeset_helper(); of_unittest_parse_interrupts(); of_unittest_parse_interrupts_extended(); of_unittest_match_node(); -- Regards, Laurent Pinchart
[PATCH v5 7/8] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
The internal LVDS encoders now have their own DT bindings. Before switching the driver infrastructure to those new bindings, implement backward-compatibility through live DT patching. Patching is disabled and will be enabled along with support for the new DT bindings in the DU driver. Signed-off-by: Laurent Pinchart --- Changes since v3: - Use the OF changeset API - Use of_graph_get_endpoint_by_regs() - Replace hardcoded constants by sizeof() Changes since v2: - Update the SPDX headers to use C-style comments in header files - Removed the manually created __local_fixups__ node - Perform manual fixups on live DT instead of overlay Changes since v1: - Select OF_FLATTREE - Compile LVDS DT bindings patch code when DRM_RCAR_LVDS is selected - Update the SPDX headers to use GPL-2.0 instead of GPL-2.0-only - Turn __dtb_rcar_du_of_lvds_(begin|end) from u8 to char - Pass void begin and end pointers to rcar_du_of_get_overlay() - Use of_get_parent() instead of accessing the parent pointer directly - Find the LVDS endpoints nodes based on the LVDS node instead of the root of the overlay - Update to the -lvds compatible string format --- drivers/gpu/drm/rcar-du/Kconfig| 2 + drivers/gpu/drm/rcar-du/Makefile | 7 +- drivers/gpu/drm/rcar-du/rcar_du_of.c | 307 + drivers/gpu/drm/rcar-du/rcar_du_of.h | 20 ++ .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts| 79 ++ .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts| 53 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts| 53 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts| 53 .../gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts| 53 9 files changed, 626 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.c create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of.h create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7791.dts create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7793.dts create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7795.dts create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7796.dts diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig index 5d0b4b7119af..3f83352a7313 100644 --- a/drivers/gpu/drm/rcar-du/Kconfig +++ b/drivers/gpu/drm/rcar-du/Kconfig @@ -22,6 +22,8 @@ config DRM_RCAR_LVDS bool "R-Car DU LVDS Encoder Support" depends on DRM_RCAR_DU select DRM_PANEL + select OF_FLATTREE + select OF_OVERLAY help Enable support for the R-Car Display Unit embedded LVDS encoders. diff --git a/drivers/gpu/drm/rcar-du/Makefile b/drivers/gpu/drm/rcar-du/Makefile index 0cf5c11030e8..86b337b4be5d 100644 --- a/drivers/gpu/drm/rcar-du/Makefile +++ b/drivers/gpu/drm/rcar-du/Makefile @@ -8,7 +8,12 @@ rcar-du-drm-y := rcar_du_crtc.o \ rcar_du_plane.o rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_lvdsenc.o - +rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)+= rcar_du_of.o \ + rcar_du_of_lvds_r8a7790.dtb.o \ + rcar_du_of_lvds_r8a7791.dtb.o \ + rcar_du_of_lvds_r8a7793.dtb.o \ + rcar_du_of_lvds_r8a7795.dtb.o \ + rcar_du_of_lvds_r8a7796.dtb.o rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c new file mode 100644 index ..12fae8814309 --- /dev/null +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * rcar_du_of.c - Legacy DT bindings compatibility + * + * Copyright (C) 2018 Laurent Pinchart + * + * Based on work from Jyri Sarha + * Copyright (C) 2015 Texas Instruments + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "rcar_du_crtc.h" +#include "rcar_du_drv.h" + +/* - + * Generic Overlay Handling + */ + +struct rcar_du_of_overlay { + const char *compatible; + void *begin; + void *end; +}; + +#define RCAR_DU_OF_DTB(type, soc) \ + extern char __dtb_rcar_du_of_##type##_##soc##_begin[]; \ + extern char __dtb_rcar_du_of_##type##_##soc##_end[] + +#define RCAR_DU_OF_OVERLAY(type, soc) \ + { \ + .compatible = "renesas,du-" #soc, \ + .begin = __dtb_rcar_du_of_##type##_##soc##_begin, \ + .end = __dtb_rcar_du_of_##type##_##soc##_
[PATCH v5 1/8] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add corresponding device tree bindings. Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- Changes since v1: - Move the SoC name before the IP name in compatible strings - Rename parallel input to parallel RGB input - Fixed "renesas,r8a7743-lvds" description - Document the resets property - Fixed typo --- .../bindings/display/bridge/renesas,lvds.txt | 56 ++ MAINTAINERS| 1 + 2 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt new file mode 100644 index ..2b19ce51ec07 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -0,0 +1,56 @@ +Renesas R-Car LVDS Encoder +== + +These DT bindings describe the LVDS encoder embedded in the Renesas R-Car +Gen2, R-Car Gen3 and RZ/G SoCs. + +Required properties: + +- compatible : Shall contain one of + - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders + - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders + - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders + - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders + - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders + - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders + +- reg: Base address and length for the memory-mapped registers +- clocks: A phandle + clock-specifier pair for the functional clock +- resets: A phandle + reset specifier for the module reset + +Required nodes: + +The LVDS encoder has two video ports. Their connections are modelled using the +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. + +- Video port 0 corresponds to the parallel RGB input +- Video port 1 corresponds to the LVDS output + +Each port shall have a single endpoint. + + +Example: + + lvds0: lvds@feb9 { + compatible = "renesas,r8a7790-lvds"; + reg = <0 0xfeb9 0 0x1c>; + clocks = <&cpg CPG_MOD 726>; + resets = <&cpg 726>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds0_in: endpoint { + remote-endpoint = <&du_out_lvds0>; + }; + }; + port@1 { + reg = <1>; + lvds0_out: endpoint { + }; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 2afba909724c..13c8ec11135a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4744,6 +4744,7 @@ F:drivers/gpu/drm/rcar-du/ F: drivers/gpu/drm/shmobile/ F: include/linux/platform_data/shmob_drm.h F: Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt +F: Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt F: Documentation/devicetree/bindings/display/renesas,du.txt DRM DRIVERS FOR ROCKCHIP -- Regards, Laurent Pinchart
Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
Hi Rob, On Thursday, 22 February 2018 01:28:48 EET Rob Herring wrote: > On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > > The internal LVDS encoders now have their own DT bindings. Before > > switching the driver infrastructure to those new bindings, implement > > backward-compatibility through live DT patching. > > > > Patching is disabled and will be enabled along with support for the new > > DT bindings in the DU driver. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > [...] > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts > > b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts new file mode > > 100644 > > index ..6ebb355b652a > > --- /dev/null > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts > > @@ -0,0 +1,81 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for > > R8A7790 > > + * > > + * Copyright (C) 2018 Laurent Pinchart > > > > + * > > + * Based on work from Jyri Sarha > > + * Copyright (C) 2015 Texas Instruments > > + */ > > + > > +#include > > Doesn't seem to be used in any of these. It's a leftover from a previous test. I'll remove it in all the .dts files. > Otherwise, > > Reviewed-by: Rob Herring > > > + > > +/dts-v1/; > > +/plugin/; > > +/ { > > + fragment@0 { > > + target-path = "/"; > > + __overlay__ { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + lvds@feb9 { > > + compatible = "renesas,r8a7790-lvds"; > > + reg = <0 0xfeb9 0 0x1c>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + lvds0_input: endpoint { > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + lvds0_out: endpoint { > > + }; > > + }; > > + }; > > + }; > > + > > + lvds@feb94000 { > > + compatible = "renesas,r8a7790-lvds"; > > + reg = <0 0xfeb94000 0 0x1c>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + lvds1_input: endpoint { > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + lvds1_out: endpoint { > > + }; > > + }; > > + }; > > + }; > > + }; > > + }; > > + > > + fragment@1 { > > + target-path = "/display@feb0/ports"; > > + __overlay__ { > > + port@1 { > > + endpoint { > > + remote-endpoint = <&lvds0_input>; > > + }; > > + }; > > + port@2 { > > + endpoint { > > + remote-endpoint = <&lvds1_input>; > > + }; > > + }; > > + }; > > + }; > > +}; -- Regards, Laurent Pinchart
Re: [PATCH v4 06/16] of: unittest: changeset helpers
On Wed, Feb 21, 2018 at 5:39 PM, Laurent Pinchart wrote: > Hi Rob, > > On Thursday, 22 February 2018 01:10:25 EET Rob Herring wrote: >> On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: >> > From: Pantelis Antoniou >> > >> > Add a unitest specific for the new changeset helpers. >> > >> > Signed-off-by: Pantelis Antoniou >> > Signed-off-by: Laurent Pinchart >> > >> > --- >> > >> > drivers/of/unittest.c | 54 ++ >> > 1 file changed, 54 insertions(+) >> > >> > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c >> > index 7a9abaae874d..1b21d2c549a8 100644 >> > --- a/drivers/of/unittest.c >> > +++ b/drivers/of/unittest.c >> > @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void) >> > >> > #endif >> > } >> > >> > +static void __init of_unittest_changeset_helper(void) >> > +{ >> > +#ifdef CONFIG_OF_DYNAMIC >> >> I think this can be: >> >> if (!IS_ENABLED(CONFIG_OF_DYNAMIC)) >> return; > > Not quite, as there are functions used below (such as of_changeset_init()) > that are not defined if CONFIG_OF_DYNAMIC isn't enabled. We could create stubs > in that case but I believe that's out of scope for this patch series. Okay. I thought we had the necessary stubs, but didn't check too closely. > >> Otherwise, >> >> Reviewed-by: Rob Herring > > -- > Regards, > > Laurent Pinchart >
Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method
Hi Rob, On Thursday, 22 February 2018 01:20:36 EET Rob Herring wrote: > On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > > From: Pantelis Antoniou > > > > Adds a changeset helper for moving a subtree to a different place > > in the running tree. This is useful in advances cases of dynamic > > device tree construction. > > This one I'm not real clear on when we'd use this and we don't have > any user, so lets drop it for now. OK, no problem. -- Regards, Laurent Pinchart
Re: [PATCH v4 06/16] of: unittest: changeset helpers
Hi Rob, On Thursday, 22 February 2018 01:10:25 EET Rob Herring wrote: > On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > > From: Pantelis Antoniou > > > > Add a unitest specific for the new changeset helpers. > > > > Signed-off-by: Pantelis Antoniou > > Signed-off-by: Laurent Pinchart > > > > --- > > > > drivers/of/unittest.c | 54 ++ > > 1 file changed, 54 insertions(+) > > > > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > > index 7a9abaae874d..1b21d2c549a8 100644 > > --- a/drivers/of/unittest.c > > +++ b/drivers/of/unittest.c > > @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void) > > > > #endif > > } > > > > +static void __init of_unittest_changeset_helper(void) > > +{ > > +#ifdef CONFIG_OF_DYNAMIC > > I think this can be: > > if (!IS_ENABLED(CONFIG_OF_DYNAMIC)) > return; Not quite, as there are functions used below (such as of_changeset_init()) that are not defined if CONFIG_OF_DYNAMIC isn't enabled. We could create stubs in that case but I believe that's out of scope for this patch series. > Otherwise, > > Reviewed-by: Rob Herring -- Regards, Laurent Pinchart
[PATCH] pinctrl: sh-pfc: r8a7795: remove duplicate of CLKOUT pin in pinmux_pins[]
When adding GP-1-28 port pin support it was forgotten to remove the CLKOUT pin from the list of pins that are not associated with a GPIO port in pinmux_pins[]. This results in a warning when reading the pinctrl files in sysfs as the CLKOUT pin is still added as a none GPIO pin. Fix this by removing the duplicated entry which is no longer needed. ~ # cat /sys/kernel/debug/pinctrl/e606.pin-controller/pinconf-pins [ 89.432081] [ cut here ] [ 89.436904] Pin 496 is not in bias info list [ 89.441252] WARNING: CPU: 1 PID: 456 at drivers/pinctrl/sh-pfc/core.c:408 sh_pfc_pin_to_bias_reg+0xb0/0xb8 [ 89.451002] CPU: 1 PID: 456 Comm: cat Not tainted 4.16.0-rc1-arm64-renesas-00048-gdfafc344a4f24dde #12 [ 89.460394] Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT) [ 89.468910] pstate: 8085 (Nzcv daIf -PAN -UAO) [ 89.473747] pc : sh_pfc_pin_to_bias_reg+0xb0/0xb8 [ 89.478495] lr : sh_pfc_pin_to_bias_reg+0xb0/0xb8 [ 89.483241] sp : 0aff3ab0 [ 89.486587] x29: 0aff3ab0 x28: 0893c698 [ 89.491955] x27: 08ad7d98 x26: [ 89.497323] x25: 8006fb3f5028 x24: 8006fb3f5018 [ 89.502690] x23: 0001 x22: 01f0 [ 89.508057] x21: 8006fb3f5018 x20: 08bef000 [ 89.513423] x19: x18: [ 89.518790] x17: 6c4a x16: 08d67c98 [ 89.524157] x15: 0001 x14: 0896ca98 [ 89.529524] x13: cce5f611 x12: 8006f8d3b5a8 [ 89.534891] x11: 0981e000 x10: 08befa08 [ 89.540258] x9 : 8006f9b987a0 x8 : 08befa08 [ 89.545625] x7 : 08137094 x6 : [ 89.550991] x5 : x4 : 0001 [ 89.556357] x3 : 0007 x2 : 0007 [ 89.561723] x1 : 1ff24f80f1818600 x0 : [ 89.567091] Call trace: [ 89.569561] sh_pfc_pin_to_bias_reg+0xb0/0xb8 [ 89.573960] r8a7795_pinmux_get_bias+0x30/0xc0 [ 89.578445] sh_pfc_pinconf_get+0x1e0/0x2d8 [ 89.582669] pin_config_get_for_pin+0x20/0x30 [ 89.587067] pinconf_generic_dump_one+0x180/0x1c8 [ 89.591815] pinconf_generic_dump_pins+0x84/0xd8 [ 89.596476] pinconf_pins_show+0xc8/0x130 [ 89.600528] seq_read+0xe4/0x510 [ 89.603789] full_proxy_read+0x60/0x90 [ 89.607576] __vfs_read+0x30/0x140 [ 89.611010] vfs_read+0x90/0x170 [ 89.614269] SyS_read+0x60/0xd8 [ 89.617443] __sys_trace_return+0x0/0x4 [ 89.621314] ---[ end trace 99c8d0d39c13e794 ]--- Fixes: 82d2de5a4f646f72 ("pinctrl: sh-pfc: r8a7795: Add GP-1-28 port pin support") Signed-off-by: Niklas Söderlund --- drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c index 18aeee592fdcf246..35951e7b89d2fae3 100644 --- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c @@ -1538,7 +1538,6 @@ static const struct sh_pfc_pin pinmux_pins[] = { SH_PFC_PIN_NAMED_CFG('B', 18, AVB_TD1, CFG_FLAGS), SH_PFC_PIN_NAMED_CFG('B', 19, AVB_RXC, CFG_FLAGS), SH_PFC_PIN_NAMED_CFG('C', 1, PRESETOUT#, CFG_FLAGS), - SH_PFC_PIN_NAMED_CFG('F', 1, CLKOUT, CFG_FLAGS), SH_PFC_PIN_NAMED_CFG('H', 37, MLB_REF, CFG_FLAGS), SH_PFC_PIN_NAMED_CFG('V', 3, QSPI1_SPCLK, CFG_FLAGS), SH_PFC_PIN_NAMED_CFG('V', 5, QSPI1_SSL, CFG_FLAGS), -- 2.16.1
Re: [PATCH v4 08/16] drm: rcar-du: Fix legacy DT to create LVDS encoder nodes
On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > The internal LVDS encoders now have their own DT bindings. Before > switching the driver infrastructure to those new bindings, implement > backward-compatibility through live DT patching. > > Patching is disabled and will be enabled along with support for the new > DT bindings in the DU driver. > > Signed-off-by: Laurent Pinchart > --- [...] > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts > b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts > new file mode 100644 > index ..6ebb355b652a > --- /dev/null > +++ b/drivers/gpu/drm/rcar-du/rcar_du_of_lvds_r8a7790.dts > @@ -0,0 +1,81 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for > R8A7790 > + * > + * Copyright (C) 2018 Laurent Pinchart > + * > + * Based on work from Jyri Sarha > + * Copyright (C) 2015 Texas Instruments > + */ > + > +#include Doesn't seem to be used in any of these. Otherwise, Reviewed-by: Rob Herring > + > +/dts-v1/; > +/plugin/; > +/ { > + fragment@0 { > + target-path = "/"; > + __overlay__ { > + #address-cells = <2>; > + #size-cells = <2>; > + > + lvds@feb9 { > + compatible = "renesas,r8a7790-lvds"; > + reg = <0 0xfeb9 0 0x1c>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds0_input: endpoint { > + }; > + }; > + port@1 { > + reg = <1>; > + lvds0_out: endpoint { > + }; > + }; > + }; > + }; > + > + lvds@feb94000 { > + compatible = "renesas,r8a7790-lvds"; > + reg = <0 0xfeb94000 0 0x1c>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds1_input: endpoint { > + }; > + }; > + port@1 { > + reg = <1>; > + lvds1_out: endpoint { > + }; > + }; > + }; > + }; > + }; > + }; > + > + fragment@1 { > + target-path = "/display@feb0/ports"; > + __overlay__ { > + port@1 { > + endpoint { > + remote-endpoint = <&lvds0_input>; > + }; > + }; > + port@2 { > + endpoint { > + remote-endpoint = <&lvds1_input>; > + }; > + }; > + }; > + }; > +};
Re: [PATCH v4 05/16] of: changeset: Add of_changeset_node_move method
On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Adds a changeset helper for moving a subtree to a different place > in the running tree. This is useful in advances cases of dynamic > device tree construction. This one I'm not real clear on when we'd use this and we don't have any user, so lets drop it for now. Rob
[PATCH] i2c: adv748x: afe: fix sparse warning
This fixes the following sparse warning: drivers/media/i2c/adv748x/adv748x-afe.c:294:34:expected unsigned int [usertype] *signal drivers/media/i2c/adv748x/adv748x-afe.c:294:34:got int * drivers/media/i2c/adv748x/adv748x-afe.c:294:34: warning: incorrect type in argument 2 (different signedness) Signed-off-by: Niklas Söderlund --- drivers/media/i2c/adv748x/adv748x-afe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/adv748x/adv748x-afe.c b/drivers/media/i2c/adv748x/adv748x-afe.c index 5188178588c9067d..39a9996d0db08c31 100644 --- a/drivers/media/i2c/adv748x/adv748x-afe.c +++ b/drivers/media/i2c/adv748x/adv748x-afe.c @@ -275,7 +275,8 @@ static int adv748x_afe_s_stream(struct v4l2_subdev *sd, int enable) { struct adv748x_afe *afe = adv748x_sd_to_afe(sd); struct adv748x_state *state = adv748x_afe_to_state(afe); - int ret, signal = V4L2_IN_ST_NO_SIGNAL; + unsigned int signal = V4L2_IN_ST_NO_SIGNAL; + int ret; mutex_lock(&state->mutex); -- 2.16.1
Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Add an __of_node_dupv() private method and make __of_node_dup() use it. > This is required for the subsequent changeset accessors which will > make use of it. > > Signed-off-by: Pantelis Antoniou > Signed-off-by: Laurent Pinchart > --- > drivers/of/dynamic.c | 29 +++-- > 1 file changed, 23 insertions(+), 6 deletions(-) Reviewed-by: Rob Herring
Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Changesets are very powerful, but the lack of a helper API > makes using them cumbersome. Introduce a simple copy based > API that makes things considerably easier. > > To wit, adding a property using the raw API. > > struct property *prop; > prop = kzalloc(sizeof(*prop)), GFP_KERNEL); > prop->name = kstrdup("compatible"); > prop->value = kstrdup("foo,bar"); > prop->length = strlen(prop->value) + 1; > of_changeset_add_property(ocs, np, prop); > > while using the helper API > > of_changeset_add_property_string(ocs, np, "compatible", > "foo,bar"); > > Signed-off-by: Pantelis Antoniou > [Fixed memory leak in __of_changeset_add_update_property_copy()] > Signed-off-by: Laurent Pinchart > --- > drivers/of/dynamic.c | 222 ++ > include/linux/of.h | 328 > +++ > 2 files changed, 550 insertions(+) Other than what Geert pointed out, Reviewed-by: Rob Herring
Re: [PATCH v4 06/16] of: unittest: changeset helpers
On Tue, Feb 20, 2018 at 5:10 PM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Add a unitest specific for the new changeset helpers. > > Signed-off-by: Pantelis Antoniou > Signed-off-by: Laurent Pinchart > --- > drivers/of/unittest.c | 54 > +++ > 1 file changed, 54 insertions(+) > > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > index 7a9abaae874d..1b21d2c549a8 100644 > --- a/drivers/of/unittest.c > +++ b/drivers/of/unittest.c > @@ -609,6 +609,59 @@ static void __init of_unittest_changeset(void) > #endif > } > > +static void __init of_unittest_changeset_helper(void) > +{ > +#ifdef CONFIG_OF_DYNAMIC I think this can be: if (!IS_ENABLED(CONFIG_OF_DYNAMIC)) return; Otherwise, Reviewed-by: Rob Herring
Re: [PATCH v10 04/10] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)
On Wed, Feb 21, 2018 at 06:47:58PM +0100, Jacopo Mondi wrote: > Add Capture Engine Unit (CEU) node to device tree. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven > Reviewed-by: Laurent Pinchart > Acked-by: Hans Verkuil This patch depends on the binding for "renesas,r7s72100-ceu". Please repost or otherwise ping me once that dependency has been accepted.
Re: [PATCH 1/2] ARM: shmobile: Factor out complex condition
On 02/19/2018 09:58 AM, Simon Horman wrote: > On Sat, Feb 17, 2018 at 03:06:41AM +0100, Marek Vasut wrote: >> Pull the complex condition in regulator_quirk_notify() into >> regulator_quirk_check(). Moreover, do not hard-code the I2C >> address, but rather use the one in da9xxx_msgs[]. >> >> Signed-off-by: Marek Vasut >> Cc: Geert Uytterhoeven >> Cc: Kuninori Morimoto >> Cc: Simon Horman >> Cc: Wolfram Sang > > There appears to be some review of this series that needs addressing, > I have marked it as "Changes Requested". > Jupp -- Best regards, Marek Vasut
Re: [PATCH v2] videodev2.h: add helper to validate colorspace
Hi Laurent and Hans, On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote: > No, I'm sorry, for MC-based drivers this isn't correct. The media entity that > symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a > subdev. It thus has no media bus format configured for its sink pad. The > closest pad in the pipeline that has a media bus format is the source pad of > the subdev connected to the video node. > > There's no communication within the kernel at G/S_FMT time between the video > node and its connected subdev. The only time we look at the pipeline as a > whole is when starting the stream to validate that the pipeline is correctly > configured. We thus have to implement G/S_FMT on the video node without any > knowledge about the connected subdev, and thus accept any colorspace. A few more notes related to this --- there's no propagation of sub-device format across the entities; there were several reasons for the design choice. The V4L2 pixel format in the video node must be compatible with the sub-device format when streaming is started. And... the streaming will start in the pipeline defined by the enabled links to/from the video node. In principle nothign prevents having multiple links there, and at the time S_FMT IOCTL is called on the video node, none of those links could be enabled. And that's perfectly valid use of the API. A lot of the DMA engine drivers are simply devices that receive data and write that to system memory, they really don't necessarily know anything else. For the hardware this data is just pixels (or even bits, especially in the case of CSI-2!). So I agree with Laurent here that requiring correct colour space for [GS]_FMT IOCTLs on video nodes in the general case is not feasible (especially on MC-centric devices), due to the way the Media controller pipeline and formats along that pipeline are configured and validated (i.e. at streamon time). But what to do here then? The colourspace field is not verified even in link validation so there's no guarantee it's correctly set more or less anywhere else than in the source of the stream. And if the stream has multiple sources with different colour spaces, then what do you do? That's perhaps a theoretical case but the current frameworks and APIs do in principle support that. Perhaps we should specify that the user should always set the same colourspace on the sink end of a link that was there in the source? The same should likely apply to the rest of the fields apart from width, height and code, too. Before the user configures formats this doesn't work though, and this does not address the matter with v4l2-compliance. Granted that the drivers will themselves handle the colour space information correctly, it'd still provide a way for the user to gain the knowledge of the colour space which I believe is what matters. -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH] clk: renesas: rcar-gen3: Fix SD divider setting
Simon, sorry for the delay in checking this. I refreshed my memory about the issues here this evening and will be able to do educated tests tomorrow. However, one question already: > - In M3N, HS400 mode and HS200 mode use the same clock setting. a) I did not find any code handling this speciality for M3N, neither here nor in the SDHI driver. Did you notice something in the BSP? b) do you know the reasons for this? Looking at the datasheet I have, sdsrc is 800MHz for all Gen3 SoC. > On H3 ES1.0 / Salvator-X I do not see any problems either with or without > this patch. I assume this is because the speed is 200MHz instead of 400MHz. The sdsrc clk is half speed on Salvator-X-H3-ES1.0. I hope I can confirm my assumption tomorrow. > https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git > topic/hs400-v2 Thanks, that was useful! Regards, Wolfram signature.asc Description: PGP signature
Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt
Hi Jacopo, On Tuesday, 20 February 2018 10:58:57 EET jacopo mondi wrote: > On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote: > > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote: > >> The sensor driver sets mbus format colorspace information and sizes, > >> but not ycbcr encoding, quantization and xfer function. When supplied > >> with an badly initialized mbus frame format structure, those fields > >> need to be set explicitly not to leave them uninitialized. This is > >> tested by v4l2-compliance, which supplies a mbus format description > >> structure and checks for all fields to be properly set. > >> > >> Without this commit, v4l2-compliance fails when testing formats with: > >> fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff > >> > >> Signed-off-by: Jacopo Mondi > >> --- > >> > >> drivers/media/i2c/ov7670.c | 4 > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c > >> index 25b26d4..61c472e 100644 > >> --- a/drivers/media/i2c/ov7670.c > >> +++ b/drivers/media/i2c/ov7670.c > >> @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct > >> v4l2_subdev > >> *sd, fmt->height = wsize->height; > >> > >>fmt->colorspace = ov7670_formats[index].colorspace; > > > > On a side note, if I'm not mistaken the colorspace field is set to SRGB > > for all entries. Shouldn't you hardcode it here and remove the field ? > > > >> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > >> + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; > >> + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; > > > > How about setting the values explicitly instead of relying on defaults ? > > That would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and > > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if > > the sensor outputs limited or full range ? > > This actually makes me wonder if those informations (ycbcb_enc, > quantization and xfer_func) shouldn't actually be part of the > supported format list... I blindly added those default fields in the > try_fmt function, but I doubt they applies to all supported formats. > > Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and > RGB 565) and 1 raw format (BGGR). I now have a question here: > > 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this > applies to RGB and raw formats? I don't think so, and what value is > the correct one for the ycbcr_enc field in this case? I assume > xfer_func and quantization applies to all formats instead.. There's no encoding for RGB formats if I understand things correctly, so I'd set ycbcr_enc to V4L2_YCBCR_ENC_DEFAULT. The transfer function and the quantization apply to all formats, but I'd be surprised to find a sensor outputting limited range for RGB. Have you been able to check whether the sensor outputs limited range of full range YUV ? If it outputs full range you can hardcode quantization to V4L2_QUANTIZATION_FULL_RANGE for all formats. > >>info->format = *fmt; > >> > >>return 0; -- Regards, Laurent Pinchart
Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1
On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote: > Hi Simon, > > On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman wrote: > > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote: > >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro > >> wrote: > >> > this series has been around for some time as RFC, and it has collected > >> > useful comments from the community along the way. > >> > The solution proposed by this patch set works for most R-Car Gen2 and > >> > RZ/G1 devices, but not all of them. We now know that for some R-Car > >> > Gen2 early revisions there is no proper software fix. Anyway, no > >> > product has been built around early revisions, but development boards > >> > mounting early revisions (basically prototypes) are still out there. > >> > As a result, this series isn't enabling the internal watchdog on R-Car > >> > Gen2 boards, developers may enable it in board specific device trees > >> > if needed. > >> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt, > >> > and Koelsch boards. > >> > > >> > The problem > >> > === > >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector > >> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON > >> > the non-boot CPUs they are redirected to the reset vector installed by > >> > Linux in ICRAM1, and eventually they continue the execution to RAM, > >> > where the SMP bring-up code will take care of the rest. > >> > The content of the [S]BAR registers survives a watchdog triggered reset, > >> > and as such after the watchdog fires the boot core will try and execute > >> > the SMP bring-up code instead of jumping to the bootrom code. > >> > > >> > The fix > >> > === > >> > The main strategy for the solution is to let the reset vector decide > >> > if it needs to jump to shmobile_boot_fn or to the bootrom code. > >> > In a watchdog triggered reset scenario, since the [S]BAR registers keep > >> > their values, the boot CPU will jump into the newly designed reset > >> > vector, the assembly routine will eventually test WOVF (a bit in register > >> > RWTCSRA that indicates if the watchdog counter has overflown, the value > >> > of this bit gets retained in this scenario), and jump to the bootrom code > >> > which will in turn load up the bootloader, etc. > >> > When bringing up SMP or using CPU hotplug, the reset vector will jump > >> > to shmobile_boot_fn instead. > >> > > >> > Thank you All for your help. > >> > > >> > Best regards, > >> > > >> > Fabrizio Castro (26): > >> > ARM: shmobile: Add watchdog support > >> > ARM: dts: r8a7743: Adjust SMP routine size > >> > ARM: dts: r8a7745: Adjust SMP routine size > >> > ARM: dts: r8a7790: Adjust SMP routine size > >> > ARM: dts: r8a7791: Adjust SMP routine size > >> > ARM: dts: r8a7792: Adjust SMP routine size > >> > ARM: dts: r8a7793: Adjust SMP routine size > >> > ARM: dts: r8a7794: Adjust SMP routine size > >> > soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2 > >> > ARM: shmobile: rcar-gen2: Add watchdog support > >> > dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support > >> > watchdog: renesas_wdt: Add R-Car Gen2 support > >> > watchdog: renesas_wdt: Add restart handler > >> > ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN > >> > clk: renesas: r8a7743: Add rwdt clock > >> > clk: renesas: r8a7745: Add rwdt clock > >> > clk: renesas: r8a7790: Add rwdt clock > >> > clk: renesas: r8a7791/r8a7793: Add rwdt clock > >> > clk: renesas: r8a7794: Add rwdt clock > >> > ARM: dts: r8a7743: Add watchdog support to SoC dtsi > >> > ARM: dts: r8a7745: Add watchdog support to SoC dtsi > >> > ARM: dts: r8a7790: Add watchdog support to SoC dtsi > >> > ARM: dts: r8a7791: Add watchdog support to SoC dtsi > >> > ARM: dts: r8a7794: Add watchdog support to SoC dtsi > >> > ARM: dts: iwg20m: Add watchdog support to SoM dtsi > >> > ARM: dts: iwg22m: Add watchdog support to SoM dtsi > >> > > >> > .../devicetree/bindings/watchdog/renesas-wdt.txt | 19 ++-- > >> > arch/arm/boot/dts/r8a7743-iwg20m.dtsi | 5 ++ > >> > arch/arm/boot/dts/r8a7743.dtsi | 12 - > >> > arch/arm/boot/dts/r8a7745-iwg22m.dtsi | 5 ++ > >> > arch/arm/boot/dts/r8a7745.dtsi | 12 - > >> > arch/arm/boot/dts/r8a7790.dtsi | 12 - > >> > arch/arm/boot/dts/r8a7791.dtsi | 12 - > >> > arch/arm/boot/dts/r8a7792.dtsi | 2 +- > >> > arch/arm/boot/dts/r8a7793.dtsi | 2 +- > >> > arch/arm/boot/dts/r8a7794.dtsi | 12 - > >> > arch/arm/configs/shmobile_defconfig| 1 + > >> > arch/arm/mach-shmobile/common.h| 6 +++ > >> > arch/arm/mach-shmobile/headsmp.S | 55 > >> > ++ > >> > arch/arm/mach-shmobile/platsmp-apmu.c | 1 + > >> >
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
On Wed, Feb 21, 2018 at 08:53:53PM +0300, Sergei Shtylyov wrote: > On 02/21/2018 08:38 PM, Sergei Shtylyov wrote: ... > >> + clocks = <&cpg CPG_MOD 812>; > >> + power-domains = <&sysc 32>; > >> + resets = <&cpg 812>; > >> + phy-mode = "rgmii-txid"; > > > >Why not just "rgmii"? TX delay is a board specific detail, no? > > > I admit I took this one straight from r8a7796 dtsi. > Would you like to me resend and change this? > >>> > >>>Yes, unless Simon would fix it while merging... > >> > >> Can I confirm the desired change is s/rgmii-txid/rgmii/ ? > > > >Yes. > >Apparently that means that this prop should be overridden in the board file > (which may not be an easy task given the board is Salvator-XS again). > > [...] Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts? I feel that I'm missing an important point here.
Re: [PATCH v2] videodev2.h: add helper to validate colorspace
Hi Hans, On Tuesday, 20 February 2018 10:37:22 EET Hans Verkuil wrote: > On 02/19/2018 11:28 PM, Niklas Söderlund wrote: > > Hi Hans, > > > > Thanks for your feedback. > > > > [snip] > > > >> Can you then fix v4l2-compliance to stop testing colorspace > >> against 0xff > >> ? > > > > For now I can simply relax this test for subdevs with sources and > > sinks. > > You also need to relax it for video nodes with MC drivers, as the DMA > engines don't care about colorspaces. > >>> > >>> Yes, they do. Many DMA engines can at least do RGB <-> YUV conversions, > >>> so they should get the colorspace info from their source and pass it on > >>> to userspace (after correcting for any conversions done by the DMA > >>> engine). > >> > >> Not in the MC case. Video nodes there only model the DMA engine, and are > >> thus not aware of colorspaces. What MC drivers do is check at stream on > >> time when validating the pipeline that the colorspace set by userspace > >> on the video node corresponds to the colorspace on the source pad of the > >> connected subdev, but that's only to ensure that userspace gets a > >> coherent view of colorspace across the pipeline, not to program the > >> hardware. There could be exceptions, but in the general case, the video > >> node implementation of an MC driver will accept any colorspace and only > >> validate it at stream on time, similarly to how it does for the frame > >> size format instance (and in the frame size case it will usually enforce > >> min/max limits when the DMA engine limits the frame size).> > > I'm afraid the issue described above by Laurent is what sparked me to > > write this commit to begin with. In my never ending VIN Gen3 patch-set I > > currency need to carry a patch [1] to implement a hack to make sure > > v4l2-compliance do not fail for the VIN Gen3 MC-centric use-case. This > > patch was an attempt to be able to validate the colorspace using the > > magic value 0xff. > > This is NOT a magic value. The test that's done here is to memset the > format structure with 0xff, then call the ioctl. Afterwards it checks > if there are any remaining 0xff bytes left in the struct since it expects > the driver to have overwritten it by something else. That's where the 0xff > comes from. It's no less or more magic than using 0xdeadbeef or any fixed value :-) I think we all agree that it isn't a value that is meant to be handled specifically by drivers, so it's not magic in that sense. > > I don't feel strongly for this patch in particular and I'm happy to drop > > it. But I would like to receive some guidance on how to then properly > > be able to handle this problem for the MC-centric VIN driver use-case. > > One option is as you suggested to relax the test in v4l-compliance to > > not check colorspace, but commit [2] is not enough to resolve the issue > > for my MC use-case. > > > > As Laurent stated above, the use-case is that the video device shall > > accept any colorspace set from user-space. This colorspace is then only > > used as stream on time to validate the MC pipeline. The VIN driver do > > not care about colorspace, but I care about not breaking v4l2-compliance > > as I find it's a very useful tool :-) > > I think part of my confusion here is that there are two places where you > deal with colorspaces in a DMA engine: first there is a input pad of the > DMA engine entity, secondly there is the v4l2_pix_format for the memory > description. > > The second is set by the driver based on what userspace specified for the > input pad, together with any changes due to additional conversions such > as quantization range and RGB <-> YUV by the DMA engine. No, I'm sorry, for MC-based drivers this isn't correct. The media entity that symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a subdev. It thus has no media bus format configured for its sink pad. The closest pad in the pipeline that has a media bus format is the source pad of the subdev connected to the video node. There's no communication within the kernel at G/S_FMT time between the video node and its connected subdev. The only time we look at the pipeline as a whole is when starting the stream to validate that the pipeline is correctly configured. We thus have to implement G/S_FMT on the video node without any knowledge about the connected subdev, and thus accept any colorspace. > So any colorspace validation is done for the input pad. The question is > what that validation should be. It's never been defined. No format is set on the video node's entity sink pad for the reason above, so no validation occurs when setting the colorspace on the sink pad as that never happens. > Also the handling of COLORSPACE_DEFAULT for pad formats needs to be defined. > > This is not the first time this cropped up, see e.g. this RFC patch: > > https://patchwork.linuxtv.org/patch/41734/ > > > I'm basing the following on the latest
Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
Hi Sergei, On Wednesday, 21 February 2018 10:35:13 EET Sergei Shtylyov wrote: > On 2/21/2018 2:10 AM, Laurent Pinchart wrote: > > The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add > > corresponding device tree bindings. > > > > Signed-off-by: Laurent Pinchart > > > > Reviewed-by: Rob Herring > > --- > > Changes since v1: > > > > - Move the SoC name before the IP name in compatible strings > > - Rename parallel input to parallel RGB input > > - Fixed "renesas,r8a7743-lvds" description > > - Document the resets property > > - Fixed typo > > --- > > > > .../bindings/display/bridge/renesas,lvds.txt | 56 > > MAINTAINERS| 1 + > > 2 files changed, 57 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt> > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt new > > file mode 100644 > > index ..2b19ce51ec07 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > > @@ -0,0 +1,56 @@ > > +Renesas R-Car LVDS Encoder > > +== > > + > > +These DT bindings describe the LVDS encoder embedded in the Renesas R-Car > > +Gen2, R-Car Gen3 and RZ/G SoCs. > > + > > +Required properties: > > + > > +- compatible : Shall contain one of > > + - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders > > + - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS > > encoders > > + - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS > > encoders + - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible > > LVDS encoders + - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) > > compatible LVDS encoders + - "renesas,r8a7796-lvds" for R8A7796 (R-Car > > M3-W) compatible LVDS encoders + > > +- reg: Base address and length for the memory-mapped registers > > +- clocks: A phandle + clock-specifier pair for the functional clock > > +- resets: A phandle + reset specifier for the module reset > > + > > +Required nodes: > > + > > +The LVDS encoder has two video ports. Their connections are modelled > > using the +OF graph bindings specified in > > Documentation/devicetree/bindings/graph.txt. + > > +- Video port 0 corresponds to the parallel RGB input > > +- Video port 1 corresponds to the LVDS output > > + > > +Each port shall have a single endpoint. > > + > > + > > +Example: > > + > > + lvds0: lvds@feb9 { > > "lvds-encoder@feb9" maybe? Or just "encoder@feb9" ? Or "display-encoder@feb9" ? Rob, any preference ? > And do we need a # in the label if the encoder is alone in DT anyway? As a matter of fact r8a7790, on which this example is based, has two LVDS encoders. I've only included one in the example as adding a second one wouldn't have added any value. > [...] -- Regards, Laurent Pinchart
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
On 02/21/2018 09:23 PM, Simon Horman wrote: > ... > + clocks = <&cpg CPG_MOD 812>; + power-domains = <&sysc 32>; + resets = <&cpg 812>; + phy-mode = "rgmii-txid"; >>> >>>Why not just "rgmii"? TX delay is a board specific detail, no? >>> >> I admit I took this one straight from r8a7796 dtsi. >> Would you like to me resend and change this? > >Yes, unless Simon would fix it while merging... Can I confirm the desired change is s/rgmii-txid/rgmii/ ? >>> >>>Yes. >> >>Apparently that means that this prop should be overridden in the board >> file >> (which may not be an easy task given the board is Salvator-XS again). >> >> [...] > > Can we override it in r8a77965-salvator-x.dts or r8a77965-salvator-xs.dts? In salvator-common.dtsi most probably -- it has the PHY data for Ether AVB. > I feel that I'm missing an important point here. Well, r8a779{5|6}.dtsi also have phy-mode = "rgmii-txid" (which was unjustified in my current understanding). Thus such board override wouldn't hurt them. But we lack a patch modifying salvator-common.dtsi in htis series, so I'm now thinking a respin of this series is needed anyway... sorry for being unclear. :-) MBR, Sergei
Re: [PATCH V3] ARM: shmobile: stout: enable R-Car Gen2 regulator quirk
On Thu, Feb 15, 2018 at 12:33:50PM +0100, Marek Vasut wrote: > Regulator setup is suboptimal on H2 Stout too. The Stout newly has > two DA9210 regulators, so the quirk is extended to handle another > DA9210 at i2c address 0x70. > > Signed-off-by: Marek Vasut > Cc: Geert Uytterhoeven > Cc: Kuninori Morimoto > Cc: Simon Horman > Cc: Wolfram Sang > --- > V2: - Handle another DA9210 at 0x70 > - Drop explicit board list from the leading comment in the file > V3: - Correct da9xxx_msgs[] array size > - Send messages to all three regulators on Stout only Thanks, applied.
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
On 02/21/2018 08:38 PM, Sergei Shtylyov wrote: >> Populate the ethernet@e680 device node to enable Ethernet interface >> for R-Car M3-N (r8a77965) SoC. >> >> Signed-off-by: Jacopo Mondi >> Reviewed-by: Geert Uytterhoeven >> >> --- >> v1 -> v2: >> - Replace ALWAYS_ON power area identifier with numeric constant >> --- >> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 >> ++- >> 1 file changed, 42 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> index 55f05f7..c249895 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> @@ -520,7 +520,48 @@ >> }; >> >> avb: ethernet@e680 { >> -/* placeholder */ >> +compatible = "renesas,etheravb-r8a77965", >> + "renesas,etheravb-rcar-gen3"; >> +reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 >> 0x1>; >> +interrupts = , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + , >> + ; >> +interrupt-names = "ch0", "ch1", "ch2", "ch3", >> + "ch4", "ch5", "ch6", "ch7", >> + "ch8", "ch9", "ch10", "ch11", >> + "ch12", "ch13", "ch14", >> "ch15", >> + "ch16", "ch17", "ch18", >> "ch19", >> + "ch20", "ch21", "ch22", >> "ch23", >> + "ch24"; >> +clocks = <&cpg CPG_MOD 812>; >> +power-domains = <&sysc 32>; >> +resets = <&cpg 812>; >> +phy-mode = "rgmii-txid"; > >Why not just "rgmii"? TX delay is a board specific detail, no? > I admit I took this one straight from r8a7796 dtsi. Would you like to me resend and change this? >>> >>>Yes, unless Simon would fix it while merging... >> >> Can I confirm the desired change is s/rgmii-txid/rgmii/ ? > >Yes. Apparently that means that this prop should be overridden in the board file (which may not be an easy task given the board is Salvator-XS again). [...] MBR, Sergei
[PATCH v10 00/10] Renesas Capture Engine Unit (CEU) V4L2 driver
Hello, 10th round, one step closer to finalize CEU inclusion (hopefully). I have dropped patch 11/11 of v9 as it is still a debated topic and I would like to have it clarified without blocking this series. In patch [3/10] Hans noticed that after changing input, format should be set on CEU again to make sure userspace can start streaming operations. While there's no clear preference if applying default formats to the newly selected input is the best option, I went for this one. While at there I renamed the "ceu_init_formats()" function to "ceu_init_mbus_fmt()" and moved field selection to "ceu_set[_default]_fmt()" functions. In patch [7/10] I have dropped two redundant memsets and closed a few warnings received from 0-day running sparse on the patch. I have declared two fields as static, and transformed an "if" clause in a "switch" one, to catch all possible cases with a "default" one and while at there replaced the frame interval array size definition with an explicit ARRAY_SIZE(). Apart from that, all patches are now acked/reviewed, apart from 03/10 that has Hans' ack still pending. Diff from v9 is so short, I captured it here for the interested ones: https://paste.debian.net/hidden/91567755 Thanks j v9->v10: - Close 0-days warning on ov772x frame interval - Set default format on CEU after input change - Drop ov7670 mbus frame format set not to block this series while topic gets clarified v8->v9: - Address Laurent's review of ov772x frame rate - Address Sergei comment on ceu node name v7->v8: - Calculate PLL divider/multiplier and do not use static tables - Change RZ/A1-H to RZ/A1H (same for L) in bindings documentation - Use rounded clock rate in Migo-R board code as SH clk_set_clk() implementation does not perform rounding - Set ycbcr_enc and other fields of v4l2_mbus_format for ov772x as patch [11/11] does for ov7670 v6->v7: - Add patch to handle ycbr_enc and other fields of v4l2_mbus_format for ov7670 - Add patch to handle frame interval for ov772x - Rebased on Hans' media-tree/parm branch with v4l2_g/s_parm_cap - Drop const modifier in CEU releated fields of Migo-R setup.c board file to silence complier warnings. v5->v6: - Add Hans' Acked-by to most patches - Fix a bad change in ov772x get_selection - Add .buf_prepare callack to CEU and verify plane sizes there - Remove VB2_USERPTR from supported io_modes in CEU driver - Remove read() fops in CEU driver v4->v5: - Added Rob's and Laurent's Reviewed-by tag to DT bindings - Change CEU driver module license to "GPL v2" to match SPDX identifier as suggested by Philippe Ombredanne - Make struct ceu_data static as suggested by Laurent and add his Reviewed-by to CEU driver. v3->v4: - Drop generic fallback compatible string "renesas,ceu" - Addressed Laurent's comments on [3/9] - Fix error messages on irq get/request - Do not leak ceudev if irq_get fails - Make irq_mask a const field v2->v3: - Improved DT bindings removing standard properties (pinctrl- ones and remote-endpoint) not specific to this driver and improved description of compatible strings - Remove ov772x's xlkc_rate property and set clock rate in Migo-R board file - Made 'xclk' clock private to ov772x driver in Migo-R board file - Change 'rstb' GPIO active output level and changed ov772x and tw9910 drivers accordingly as suggested by Fabio - Minor changes in CEU driver to address Laurent's comments - Moved Migo-R setup patch to the end of the series to silence 0-day bot - Renamed tw9910 clock to 'xti' as per video decoder manual - Changed all SPDX identifiers to GPL-2.0 from previous GPL-2.0+ v1->v2: - DT -- Addressed Geert's comments and added clocks for CEU to mstp6 clock source -- Specified supported generic video iterfaces properties in dt-bindings and simplified example - CEU driver -- Re-worked interrupt handler, interrupt management, reset(*) and capture start operation -- Re-worked querycap/enum_input/enum_frameintervals to fix some v4l2_compliance failures -- Removed soc_camera legacy operations g/s_mbus_format -- Update to new notifier implementation -- Fixed several comments from Hans, Laurent and Sakari - Migo-R -- Register clocks and gpios for sensor drivers in Migo-R setup -- Updated sensors (tw9910 and ov772x) drivers headers and drivers to close remarks from Hans and Laurent: --- Removed platform callbacks and handle clocks and gpios from sensor drivers --- Remove g/s_mbus_config operations Jacopo Mondi (10): dt-bindings: media: Add Renesas CEU bindings include: media: Add Renesas CEU driver interface media: platform: Add Renesas CEU driver ARM: dts: r7s72100: Add Capture Engine Unit (CEU) media: i2c: Copy ov772x soc_camera sensor driver media: i2c: ov772x: Remove soc_camera dependencies media: i2c: ov772x: Support frame interval handling media: i2c: Copy tw9910 soc_camera sensor driver media: i2c: tw9910: Remove soc_camera dependencies arch: sh: migor: Use new renesas-ceu camera drive
[PATCH v10 01/10] dt-bindings: media: Add Renesas CEU bindings
Add bindings documentation for Renesas Capture Engine Unit (CEU). Signed-off-by: Jacopo Mondi Reviewed-by: Rob Herring Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- .../devicetree/bindings/media/renesas,ceu.txt | 81 ++ 1 file changed, 81 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt b/Documentation/devicetree/bindings/media/renesas,ceu.txt new file mode 100644 index 000..3fc66df --- /dev/null +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt @@ -0,0 +1,81 @@ +Renesas Capture Engine Unit (CEU) +-- + +The Capture Engine Unit is the image capture interface found in the Renesas +SH Mobile and RZ SoCs. + +The interface supports a single parallel input with data bus width of 8 or 16 +bits. + +Required properties: +- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in RZ/A1H + and RZ/A1M SoCs. +- reg: Registers address base and size. +- interrupts: The interrupt specifier. + +The CEU supports a single parallel input and should contain a single 'port' +subnode with a single 'endpoint'. Connection to input devices are modeled +according to the video interfaces OF bindings specified in: +Documentation/devicetree/bindings/media/video-interfaces.txt + +Optional endpoint properties applicable to parallel input bus described in +the above mentioned "video-interfaces.txt" file are supported. + +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH respectively. + If property is not present, default is active high. +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH respectively. + If property is not present, default is active high. + +Example: + +The example describes the connection between the Capture Engine Unit and an +OV7670 image sensor connected to i2c1 interface. + +ceu: ceu@e821 { + reg = <0xe821 0x209c>; + compatible = "renesas,r7s72100-ceu"; + interrupts = ; + + pinctrl-names = "default"; + pinctrl-0 = <&vio_pins>; + + status = "okay"; + + port { + ceu_in: endpoint { + remote-endpoint = <&ov7670_out>; + + hsync-active = <1>; + vsync-active = <0>; + }; + }; +}; + +i2c1: i2c@fcfee400 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c1_pins>; + + status = "okay"; + + clock-frequency = <10>; + + ov7670: camera@21 { + compatible = "ovti,ov7670"; + reg = <0x21>; + + pinctrl-names = "default"; + pinctrl-0 = <&vio_pins>; + + reset-gpios = <&port3 11 GPIO_ACTIVE_LOW>; + powerdown-gpios = <&port3 12 GPIO_ACTIVE_HIGH>; + + port { + ov7670_out: endpoint { + remote-endpoint = <&ceu_in>; + + hsync-active = <1>; + vsync-active = <0>; + }; + }; + }; +}; -- 2.7.4
[PATCH v10 02/10] include: media: Add Renesas CEU driver interface
Add renesas-ceu header file. Do not remove the existing sh_mobile_ceu.h one as long as the original driver does not go away. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- include/media/drv-intf/renesas-ceu.h | 26 ++ 1 file changed, 26 insertions(+) create mode 100644 include/media/drv-intf/renesas-ceu.h diff --git a/include/media/drv-intf/renesas-ceu.h b/include/media/drv-intf/renesas-ceu.h new file mode 100644 index 000..52841d1 --- /dev/null +++ b/include/media/drv-intf/renesas-ceu.h @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * renesas-ceu.h - Renesas CEU driver interface + * + * Copyright 2017-2018 Jacopo Mondi + */ + +#ifndef __MEDIA_DRV_INTF_RENESAS_CEU_H__ +#define __MEDIA_DRV_INTF_RENESAS_CEU_H__ + +#define CEU_MAX_SUBDEVS2 + +struct ceu_async_subdev { + unsigned long flags; + unsigned char bus_width; + unsigned char bus_shift; + unsigned int i2c_adapter_id; + unsigned int i2c_address; +}; + +struct ceu_platform_data { + unsigned int num_subdevs; + struct ceu_async_subdev subdevs[CEU_MAX_SUBDEVS]; +}; + +#endif /* ___MEDIA_DRV_INTF_RENESAS_CEU_H__ */ -- 2.7.4
[PATCH v10 04/10] ARM: dts: r7s72100: Add Capture Engine Unit (CEU)
Add Capture Engine Unit (CEU) node to device tree. Signed-off-by: Jacopo Mondi Reviewed-by: Geert Uytterhoeven Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- arch/arm/boot/dts/r7s72100.dtsi | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi index ab9645a..23e05ce 100644 --- a/arch/arm/boot/dts/r7s72100.dtsi +++ b/arch/arm/boot/dts/r7s72100.dtsi @@ -135,9 +135,9 @@ #clock-cells = <1>; compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks"; reg = <0xfcfe042c 4>; - clocks = <&p0_clk>; - clock-indices = ; - clock-output-names = "rtc"; + clocks = <&b_clk>, <&p0_clk>; + clock-indices = ; + clock-output-names = "ceu", "rtc"; }; mstp7_clks: mstp7_clks@fcfe0430 { @@ -667,4 +667,13 @@ power-domains = <&cpg_clocks>; status = "disabled"; }; + + ceu: camera@e821 { + reg = <0xe821 0x3000>; + compatible = "renesas,r7s72100-ceu"; + interrupts = ; + clocks = <&mstp6_clks R7S72100_CLK_CEU>; + power-domains = <&cpg_clocks>; + status = "disabled"; + }; }; -- 2.7.4
[PATCH v10 03/10] media: platform: Add Renesas CEU driver
Add driver for Renesas Capture Engine Unit (CEU). The CEU interface supports capturing 'data' (YUV422) and 'images' (NV[12|21|16|61]). This driver aims to replace the soc_camera-based sh_mobile_ceu one. Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ platform GR-Peach. Tested with ov7725 camera sensor on SH4 platform Migo-R. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- drivers/media/platform/Kconfig |9 + drivers/media/platform/Makefile |1 + drivers/media/platform/renesas-ceu.c | 1670 ++ 3 files changed, 1680 insertions(+) create mode 100644 drivers/media/platform/renesas-ceu.c diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig index 614fbef..f9cc058 100644 --- a/drivers/media/platform/Kconfig +++ b/drivers/media/platform/Kconfig @@ -144,6 +144,15 @@ config VIDEO_STM32_DCMI To compile this driver as a module, choose M here: the module will be called stm32-dcmi. +config VIDEO_RENESAS_CEU + tristate "Renesas Capture Engine Unit (CEU) driver" + depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA + depends on ARCH_SHMOBILE || ARCH_R7S72100 || COMPILE_TEST + select VIDEOBUF2_DMA_CONTIG + select V4L2_FWNODE + ---help--- + This is a v4l2 driver for the Renesas CEU Interface + source "drivers/media/platform/soc_camera/Kconfig" source "drivers/media/platform/exynos4-is/Kconfig" source "drivers/media/platform/am437x/Kconfig" diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile index 7f30804..85e1121 100644 --- a/drivers/media/platform/Makefile +++ b/drivers/media/platform/Makefile @@ -62,6 +62,7 @@ obj-$(CONFIG_VIDEO_SH_VOU)+= sh_vou.o obj-$(CONFIG_SOC_CAMERA) += soc_camera/ obj-$(CONFIG_VIDEO_RCAR_DRIF) += rcar_drif.o +obj-$(CONFIG_VIDEO_RENESAS_CEU)+= renesas-ceu.o obj-$(CONFIG_VIDEO_RENESAS_FCP)+= rcar-fcp.o obj-$(CONFIG_VIDEO_RENESAS_FDP1) += rcar_fdp1.o obj-$(CONFIG_VIDEO_RENESAS_JPU)+= rcar_jpu.o diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c new file mode 100644 index 000..6624fba --- /dev/null +++ b/drivers/media/platform/renesas-ceu.c @@ -0,0 +1,1670 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * V4L2 Driver for Renesas Capture Engine Unit (CEU) interface + * Copyright (C) 2017-2018 Jacopo Mondi + * + * Based on soc-camera driver "soc_camera/sh_mobile_ceu_camera.c" + * Copyright (C) 2008 Magnus Damm + * + * Based on V4L2 Driver for PXA camera host - "pxa_camera.c", + * Copyright (C) 2006, Sascha Hauer, Pengutronix + * Copyright (C) 2008, Guennadi Liakhovetski + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define DRIVER_NAME"renesas-ceu" + +/* CEU registers offsets and masks. */ +#define CEU_CAPSR 0x00 /* Capture start register */ +#define CEU_CAPCR 0x04 /* Capture control register*/ +#define CEU_CAMCR 0x08 /* Capture interface control register */ +#define CEU_CAMOR 0x10 /* Capture interface offset register */ +#define CEU_CAPWR 0x14 /* Capture interface width register*/ +#define CEU_CAIFR 0x18 /* Capture interface input format register */ +#define CEU_CRCNTR 0x28 /* CEU register control register */ +#define CEU_CRCMPR 0x2c /* CEU register forcible control register */ +#define CEU_CFLCR 0x30 /* Capture filter control register */ +#define CEU_CFSZR 0x34 /* Capture filter size clip register */ +#define CEU_CDWDR 0x38 /* Capture destination width register */ +#define CEU_CDAYR 0x3c /* Capture data address Y register */ +#define CEU_CDACR 0x40 /* Capture data address C register */ +#define CEU_CFWCR 0x5c /* Firewall operation control register */ +#define CEU_CDOCR 0x64 /* Capture data output control register*/ +#define CEU_CEIER 0x70 /* Capture event interrupt enable register */ +#define CEU_CETCR 0x74 /* Capture event flag clear register */ +#define CEU_CSTSR 0x7c /* Capture status register */ +#define CEU_CSRTR 0x80 /* Capture software reset register */ + +/* Data synchronous fetch mode. */ +#define CEU_CAMCR_JPEG BIT(4) + +/* Input components ordering: CEU_CAMCR.DTARY field. */ +#define CEU_CAMCR_DTARY_8_UYVY (0x00 << 8) +#define CEU_CAMCR_DTARY_8_VYUY (0x01 << 8) +#define CEU_CAMCR_DTARY_8_YUYV (0x02 << 8) +#define CEU_CAMCR_DTARY_8_YVYU (0x03 << 8) +/* TODO: input components orderi
[PATCH v10 07/10] media: i2c: ov772x: Support frame interval handling
Add support to ov772x driver for frame intervals handling and enumeration. Tested with 10MHz and 24MHz input clock at VGA and QVGA resolutions for 10, 15 and 30 frame per second rates. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- drivers/media/i2c/ov772x.c | 213 + 1 file changed, 196 insertions(+), 17 deletions(-) diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 23106d7..3edf0cb 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -250,6 +250,7 @@ #define AEC_1p2 0x10 /* 01: 1/2 window */ #define AEC_1p4 0x20 /* 10: 1/4 window */ #define AEC_2p3 0x30 /* 11: Low 2/3 window */ +#define COM4_RESERVED 0x01 /* Reserved bit */ /* COM5 */ #define AFR_ON_OFF 0x80 /* Auto frame rate control ON/OFF selection */ @@ -267,6 +268,10 @@ /* AEC max step control */ #define AEC_NO_LIMIT0x01 /* 0 : AEC incease step has limit */ /* 1 : No limit to AEC increase step */ +/* CLKRC */ + /* Input clock divider register */ +#define CLKRC_RESERVED 0x80 /* Reserved bit */ +#define CLKRC_DIV(n)((n) - 1) /* COM7 */ /* SCCB Register Reset */ @@ -373,6 +378,19 @@ #define VERSION(pid, ver) ((pid<<8)|(ver&0xFF)) /* + * PLL multipliers + */ +static struct { + unsigned int mult; + u8 com4; +} ov772x_pll[] = { + { 1, PLL_BYPASS, }, + { 4, PLL_4x, }, + { 6, PLL_6x, }, + { 8, PLL_8x, }, +}; + +/* * struct */ @@ -388,6 +406,7 @@ struct ov772x_color_format { struct ov772x_win_size { char *name; unsigned char com7_bit; + unsigned int sizeimage; struct v4l2_rect rect; }; @@ -404,6 +423,7 @@ struct ov772x_priv { unsigned shortflag_hflip:1; /* band_filter = COM8[5] ? 256 - BDBASE : 0 */ unsigned shortband_filter; + unsigned int fps; }; /* @@ -487,27 +507,34 @@ static const struct ov772x_color_format ov772x_cfmts[] = { static const struct ov772x_win_size ov772x_win_sizes[] = { { - .name = "VGA", - .com7_bit = SLCT_VGA, + .name = "VGA", + .com7_bit = SLCT_VGA, + .sizeimage = 510 * 748, .rect = { - .left = 140, - .top = 14, - .width = VGA_WIDTH, - .height = VGA_HEIGHT, + .left = 140, + .top= 14, + .width = VGA_WIDTH, + .height = VGA_HEIGHT, }, }, { - .name = "QVGA", - .com7_bit = SLCT_QVGA, + .name = "QVGA", + .com7_bit = SLCT_QVGA, + .sizeimage = 278 * 576, .rect = { - .left = 252, - .top = 6, - .width = QVGA_WIDTH, - .height = QVGA_HEIGHT, + .left = 252, + .top= 6, + .width = QVGA_WIDTH, + .height = QVGA_HEIGHT, }, }, }; /* + * frame rate settings lists + */ +static unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 }; + +/* * general function */ @@ -574,6 +601,128 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int enable) return 0; } +static int ov772x_set_frame_rate(struct ov772x_priv *priv, +struct v4l2_fract *tpf, +const struct ov772x_color_format *cfmt, +const struct ov772x_win_size *win) +{ + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); + unsigned long fin = clk_get_rate(priv->clk); + unsigned int fps = tpf->numerator ? + tpf->denominator / tpf->numerator : + tpf->denominator; + unsigned int best_diff; + unsigned int fsize; + unsigned int pclk; + unsigned int diff; + unsigned int idx; + unsigned int i; + u8 clkrc = 0; + u8 com4 = 0; + int ret; + + /* Approximate to the closest supported frame interval. */ + best_diff = ~0L; + for (i = 0, idx = 0; i < ARRAY_SIZE(ov772x_frame_intervals); i++) { + diff = abs(fps - ov772x_frame_intervals[i]); + if (diff < best_diff) { + idx = i; + best_diff = diff; + } + } + fps = ov772x_frame_intervals[idx]; + + /* Use image size (with bla
[PATCH v10 06/10] media: i2c: ov772x: Remove soc_camera dependencies
Remove soc_camera framework dependencies from ov772x sensor driver. - Handle clock and gpios - Register async subdevice - Remove soc_camera specific g/s_mbus_config operations - Change image format colorspace from JPEG to SRGB as the two use the same colorspace information but JPEG makes assumptions on color components quantization that do not apply to the sensor - Remove sizes crop from get_selection as driver can't scale - Add kernel doc to driver interface header file - Adjust build system This commit does not remove the original soc_camera based driver as long as other platforms depends on soc_camera-based CEU driver. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- drivers/media/i2c/Kconfig | 11 +++ drivers/media/i2c/Makefile | 1 + drivers/media/i2c/ov772x.c | 172 ++--- include/media/i2c/ov772x.h | 6 +- 4 files changed, 133 insertions(+), 57 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 9f18cd2..e71e968 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -645,6 +645,17 @@ config VIDEO_OV5670 To compile this driver as a module, choose M here: the module will be called ov5670. +config VIDEO_OV772X + tristate "OmniVision OV772x sensor support" + depends on I2C && VIDEO_V4L2 + depends on MEDIA_CAMERA_SUPPORT + ---help--- + This is a Video4Linux2 sensor-level driver for the OmniVision + OV772x camera. + + To compile this driver as a module, choose M here: the + module will be called ov772x. + config VIDEO_OV7640 tristate "OmniVision OV7640 sensor support" depends on I2C && VIDEO_V4L2 diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index c0f94cd..b0489a1 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o obj-$(CONFIG_VIDEO_OV6650) += ov6650.o obj-$(CONFIG_VIDEO_OV7640) += ov7640.o obj-$(CONFIG_VIDEO_OV7670) += ov7670.o +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o obj-$(CONFIG_VIDEO_OV7740) += ov7740.o obj-$(CONFIG_VIDEO_OV9650) += ov9650.o obj-$(CONFIG_VIDEO_OV13858) += ov13858.o diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index 8063835..23106d7 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -1,6 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * ov772x Camera Driver * + * Copyright (C) 2017 Jacopo Mondi + * * Copyright (C) 2008 Renesas Solutions Corp. * Kuninori Morimoto * @@ -9,27 +12,25 @@ * Copyright 2006-7 Jonathan Corbet * Copyright (C) 2008 Magnus Damm * Copyright (C) 2008, Guennadi Liakhovetski - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ +#include +#include +#include +#include #include #include #include -#include #include -#include #include #include #include -#include -#include + #include -#include +#include #include +#include /* * register offset @@ -393,8 +394,10 @@ struct ov772x_win_size { struct ov772x_priv { struct v4l2_subdevsubdev; struct v4l2_ctrl_handler hdl; - struct v4l2_clk *clk; + struct clk *clk; struct ov772x_camera_info*info; + struct gpio_desc *pwdn_gpio; + struct gpio_desc *rstb_gpio; const struct ov772x_color_format *cfmt; const struct ov772x_win_size *win; unsigned shortflag_vflip:1; @@ -409,7 +412,7 @@ struct ov772x_priv { static const struct ov772x_color_format ov772x_cfmts[] = { { .code = MEDIA_BUS_FMT_YUYV8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace = V4L2_COLORSPACE_SRGB, .dsp3 = 0x0, .dsp4 = DSP_OFMT_YUV, .com3 = SWAP_YUV, @@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { }, { .code = MEDIA_BUS_FMT_YVYU8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace = V4L2_COLORSPACE_SRGB, .dsp3 = UV_ON, .dsp4 = DSP_OFMT_YUV, .com3 = SWAP_YUV, @@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { }, { .code = MEDIA_BUS_FMT_UYVY8_2X8, - .colorspace = V4L2_COLORSPACE_JPEG, + .colorspace = V4L2_COLORSPACE_SRGB, .dsp3 = 0x0, .dsp4 = DSP_OFMT_YUV, .com3 = 0x0, @@ -550,7 +553,7 @@ st
[PATCH v10 05/10] media: i2c: Copy ov772x soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory. This commit just copies the original file without modifying it. No modification to KConfig and Makefile as soc_camera framework dependencies need to be removed first in next commit. Signed-off-by: Jacopo Mondi Acked-by: Laurent Pinchart Acked-by: Hans Verkuil --- drivers/media/i2c/ov772x.c | 1124 1 file changed, 1124 insertions(+) create mode 100644 drivers/media/i2c/ov772x.c diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c new file mode 100644 index 000..8063835 --- /dev/null +++ b/drivers/media/i2c/ov772x.c @@ -0,0 +1,1124 @@ +/* + * ov772x Camera Driver + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov7670 and soc_camera_platform driver, + * + * Copyright 2006-7 Jonathan Corbet + * Copyright (C) 2008 Magnus Damm + * Copyright (C) 2008, Guennadi Liakhovetski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +/* + * register offset + */ +#define GAIN0x00 /* AGC - Gain control gain setting */ +#define BLUE0x01 /* AWB - Blue channel gain setting */ +#define RED 0x02 /* AWB - Red channel gain setting */ +#define GREEN 0x03 /* AWB - Green channel gain setting */ +#define COM10x04 /* Common control 1 */ +#define BAVG0x05 /* U/B Average Level */ +#define GAVG0x06 /* Y/Gb Average Level */ +#define RAVG0x07 /* V/R Average Level */ +#define AECH0x08 /* Exposure Value - AEC MSBs */ +#define COM20x09 /* Common control 2 */ +#define PID 0x0A /* Product ID Number MSB */ +#define VER 0x0B /* Product ID Number LSB */ +#define COM30x0C /* Common control 3 */ +#define COM40x0D /* Common control 4 */ +#define COM50x0E /* Common control 5 */ +#define COM60x0F /* Common control 6 */ +#define AEC 0x10 /* Exposure Value */ +#define CLKRC 0x11 /* Internal clock */ +#define COM70x12 /* Common control 7 */ +#define COM80x13 /* Common control 8 */ +#define COM90x14 /* Common control 9 */ +#define COM10 0x15 /* Common control 10 */ +#define REG16 0x16 /* Register 16 */ +#define HSTART 0x17 /* Horizontal sensor size */ +#define HSIZE 0x18 /* Horizontal frame (HREF column) end high 8-bit */ +#define VSTART 0x19 /* Vertical frame (row) start high 8-bit */ +#define VSIZE 0x1A /* Vertical sensor size */ +#define PSHFT 0x1B /* Data format - pixel delay select */ +#define MIDH0x1C /* Manufacturer ID byte - high */ +#define MIDL0x1D /* Manufacturer ID byte - low */ +#define LAEC0x1F /* Fine AEC value */ +#define COM11 0x20 /* Common control 11 */ +#define BDBASE 0x22 /* Banding filter Minimum AEC value */ +#define DBSTEP 0x23 /* Banding filter Maximum Setp */ +#define AEW 0x24 /* AGC/AEC - Stable operating region (upper limit) */ +#define AEB 0x25 /* AGC/AEC - Stable operating region (lower limit) */ +#define VPT 0x26 /* AGC/AEC Fast mode operating region */ +#define REG28 0x28 /* Register 28 */ +#define HOUTSIZE0x29 /* Horizontal data output size MSBs */ +#define EXHCH 0x2A /* Dummy pixel insert MSB */ +#define EXHCL 0x2B /* Dummy pixel insert LSB */ +#define VOUTSIZE0x2C /* Vertical data output size MSBs */ +#define ADVFL 0x2D /* LSB of insert dummy lines in Vertical direction */ +#define ADVFH 0x2E /* MSG of insert dummy lines in Vertical direction */ +#define YAVE0x2F /* Y/G Channel Average value */ +#define LUMHTH 0x30 /* Histogram AEC/AGC Luminance high level threshold */ +#define LUMLTH 0x31 /* Histogram AEC/AGC Luminance low level threshold */ +#define HREF0x32 /* Image start and size control */ +#define DM_LNL 0x33 /* Dummy line low 8 bits */ +#define DM_LNH 0x34 /* Dummy line high 8 bits */ +#define ADOFF_B 0x35 /* AD offset compensation value for B channel */ +#define ADOFF_R 0x36 /* AD offset compensation value for R channel */ +#define ADOFF_GB0x37 /* AD offset compensation value for Gb channel */ +#define ADOFF_GR0x38 /* AD offset compensation value for Gr channel */ +#define OFF_B 0x39 /* Analog process B channel offset value */ +#define OFF_R 0x3A /* Analog process R channel offset value */ +#define OFF_GB 0x3B /* Analog process Gb channel offset value */ +#define OFF_GR 0x3C /* Analog process Gr channel offset value */ +#define COM12 0x3D /* Common control 12 */ +#define COM13 0x3E /* Common control 13 */ +#define COM14 0x3F /* Common
[PATCH v10 08/10] media: i2c: Copy tw9910 soc_camera sensor driver
Copy the soc_camera based driver in v4l2 sensor driver directory. This commit just copies the original file without modifying it. No modification to KConfig and Makefile as soc_camera framework dependencies need to be removed first in next commit. Signed-off-by: Jacopo Mondi Acked-by: Laurent Pinchart Acked-by: Hans Verkuil --- drivers/media/i2c/tw9910.c | 999 + 1 file changed, 999 insertions(+) create mode 100644 drivers/media/i2c/tw9910.c diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c new file mode 100644 index 000..bdb5e0a --- /dev/null +++ b/drivers/media/i2c/tw9910.c @@ -0,0 +1,999 @@ +/* + * tw9910 Video Driver + * + * Copyright (C) 2008 Renesas Solutions Corp. + * Kuninori Morimoto + * + * Based on ov772x driver, + * + * Copyright (C) 2008 Kuninori Morimoto + * Copyright 2006-7 Jonathan Corbet + * Copyright (C) 2008 Magnus Damm + * Copyright (C) 2008, Guennadi Liakhovetski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#define GET_ID(val) ((val & 0xF8) >> 3) +#define GET_REV(val) (val & 0x07) + +/* + * register offset + */ +#define ID 0x00 /* Product ID Code Register */ +#define STATUS10x01 /* Chip Status Register I */ +#define INFORM 0x02 /* Input Format */ +#define OPFORM 0x03 /* Output Format Control Register */ +#define DLYCTR 0x04 /* Hysteresis and HSYNC Delay Control */ +#define OUTCTR10x05 /* Output Control I */ +#define ACNTL1 0x06 /* Analog Control Register 1 */ +#define CROP_HI0x07 /* Cropping Register, High */ +#define VDELAY_LO 0x08 /* Vertical Delay Register, Low */ +#define VACTIVE_LO 0x09 /* Vertical Active Register, Low */ +#define HDELAY_LO 0x0A /* Horizontal Delay Register, Low */ +#define HACTIVE_LO 0x0B /* Horizontal Active Register, Low */ +#define CNTRL1 0x0C /* Control Register I */ +#define VSCALE_LO 0x0D /* Vertical Scaling Register, Low */ +#define SCALE_HI 0x0E /* Scaling Register, High */ +#define HSCALE_LO 0x0F /* Horizontal Scaling Register, Low */ +#define BRIGHT 0x10 /* BRIGHTNESS Control Register */ +#define CONTRAST 0x11 /* CONTRAST Control Register */ +#define SHARPNESS 0x12 /* SHARPNESS Control Register I */ +#define SAT_U 0x13 /* Chroma (U) Gain Register */ +#define SAT_V 0x14 /* Chroma (V) Gain Register */ +#define HUE0x15 /* Hue Control Register */ +#define CORING10x17 +#define CORING20x18 /* Coring and IF compensation */ +#define VBICNTL0x19 /* VBI Control Register */ +#define ACNTL2 0x1A /* Analog Control 2 */ +#define OUTCTR20x1B /* Output Control 2 */ +#define SDT0x1C /* Standard Selection */ +#define SDTR 0x1D /* Standard Recognition */ +#define TEST 0x1F /* Test Control Register */ +#define CLMPG 0x20 /* Clamping Gain */ +#define IAGC 0x21 /* Individual AGC Gain */ +#define AGCGAIN0x22 /* AGC Gain */ +#define PEAKWT 0x23 /* White Peak Threshold */ +#define CLMPL 0x24 /* Clamp level */ +#define SYNCT 0x25 /* Sync Amplitude */ +#define MISSCNT0x26 /* Sync Miss Count Register */ +#define PCLAMP 0x27 /* Clamp Position Register */ +#define VCNTL1 0x28 /* Vertical Control I */ +#define VCNTL2 0x29 /* Vertical Control II */ +#define CKILL 0x2A /* Color Killer Level Control */ +#define COMB 0x2B /* Comb Filter Control */ +#define LDLY 0x2C /* Luma Delay and H Filter Control */ +#define MISC1 0x2D /* Miscellaneous Control I */ +#define LOOP 0x2E /* LOOP Control Register */ +#define MISC2 0x2F /* Miscellaneous Control II */ +#define MVSN 0x30 /* Macrovision Detection */ +#define STATUS20x31 /* Chip STATUS II */ +#define HFREF 0x32 /* H monitor */ +#define CLMD 0x33 /* CLAMP MODE */ +#define IDCNTL 0x34 /* ID Detection Control */ +#define CLCNTL10x35 /* Clamp Control I */ +#define ANAPLLCTL 0x4C +#define VBIMIN 0x4D +#define HSLOWCTL 0x4E +#define WSS3 0x4F +#define FILLDATA 0x50 +#define SDID 0x51 +#define DID0x52 +#define WSS1 0x53 +#define WSS2 0x54 +#define VVBI 0x55 +#define LCTL6 0x56 +#define LCTL7 0x57 +#define LCTL8 0x58 +#define LCTL9 0x59 +#define LCTL10 0x5A +#define LCTL11 0x5B +#define LCTL12 0x5C +#define LCTL13 0x5D +#define LC
[PATCH v10 09/10] media: i2c: tw9910: Remove soc_camera dependencies
Remove soc_camera framework dependencies from tw9910 sensor driver. - Handle clock and gpios - Register async subdevice - Remove soc_camera specific g/s_mbus_config operations - Add kernel doc to driver interface header file - Adjust build system This commit does not remove the original soc_camera based driver as long as other platforms depends on soc_camera-based CEU driver. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- drivers/media/i2c/Kconfig | 9 +++ drivers/media/i2c/Makefile | 1 + drivers/media/i2c/tw9910.c | 162 - include/media/i2c/tw9910.h | 9 +++ 4 files changed, 120 insertions(+), 61 deletions(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index e71e968..0460613 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -423,6 +423,15 @@ config VIDEO_TW9906 To compile this driver as a module, choose M here: the module will be called tw9906. +config VIDEO_TW9910 + tristate "Techwell TW9910 video decoder" + depends on VIDEO_V4L2 && I2C + ---help--- + Support for Techwell TW9910 NTSC/PAL/SECAM video decoder. + + To compile this driver as a module, choose M here: the + module will be called tw9910. + config VIDEO_VPX3220 tristate "vpx3220a, vpx3216b & vpx3214c video decoders" depends on VIDEO_V4L2 && I2C diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile index b0489a1..23c3ac6 100644 --- a/drivers/media/i2c/Makefile +++ b/drivers/media/i2c/Makefile @@ -48,6 +48,7 @@ obj-$(CONFIG_VIDEO_TVP7002) += tvp7002.o obj-$(CONFIG_VIDEO_TW2804) += tw2804.o obj-$(CONFIG_VIDEO_TW9903) += tw9903.o obj-$(CONFIG_VIDEO_TW9906) += tw9906.o +obj-$(CONFIG_VIDEO_TW9910) += tw9910.o obj-$(CONFIG_VIDEO_CS3308) += cs3308.o obj-$(CONFIG_VIDEO_CS5345) += cs5345.o obj-$(CONFIG_VIDEO_CS53L32A) += cs53l32a.o diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c index bdb5e0a..96792df 100644 --- a/drivers/media/i2c/tw9910.c +++ b/drivers/media/i2c/tw9910.c @@ -1,6 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * tw9910 Video Driver * + * Copyright (C) 2017 Jacopo Mondi + * * Copyright (C) 2008 Renesas Solutions Corp. * Kuninori Morimoto * @@ -10,12 +13,10 @@ * Copyright 2006-7 Jonathan Corbet * Copyright (C) 2008 Magnus Damm * Copyright (C) 2008, Guennadi Liakhovetski - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ +#include +#include #include #include #include @@ -25,9 +26,7 @@ #include #include -#include #include -#include #include #define GET_ID(val) ((val & 0xF8) >> 3) @@ -228,8 +227,10 @@ struct tw9910_scale_ctrl { struct tw9910_priv { struct v4l2_subdev subdev; - struct v4l2_clk *clk; + struct clk *clk; struct tw9910_video_info*info; + struct gpio_desc*pdn_gpio; + struct gpio_desc*rstb_gpio; const struct tw9910_scale_ctrl *scale; v4l2_std_id norm; u32 revision; @@ -582,13 +583,66 @@ static int tw9910_s_register(struct v4l2_subdev *sd, } #endif +static int tw9910_power_on(struct tw9910_priv *priv) +{ + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); + int ret; + + if (priv->clk) { + ret = clk_prepare_enable(priv->clk); + if (ret) + return ret; + } + + if (priv->pdn_gpio) { + gpiod_set_value(priv->pdn_gpio, 0); + usleep_range(500, 1000); + } + + /* +* FIXME: The reset signal is connected to a shared GPIO on some +* platforms (namely the SuperH Migo-R). Until a framework becomes +* available to handle this cleanly, request the GPIO temporarily +* to avoid conflicts. +*/ + priv->rstb_gpio = gpiod_get_optional(&client->dev, "rstb", +GPIOD_OUT_LOW); + if (IS_ERR(priv->rstb_gpio)) { + dev_info(&client->dev, "Unable to get GPIO \"rstb\""); + return PTR_ERR(priv->rstb_gpio); + } + + if (priv->rstb_gpio) { + gpiod_set_value(priv->rstb_gpio, 1); + usleep_range(500, 1000); + gpiod_set_value(priv->rstb_gpio, 0); + usleep_range(500, 1000); + + gpiod_put(priv->rstb_gpio); + } + + return 0; +} + +static int tw9910_power_off(struct tw9910_priv *priv) +{ + clk_disable_unprepare(priv->clk); + + if (priv->pdn_gpio) { + gpiod_set_value(priv->pdn_gpio, 1); + usleep_range(500, 1000); + } + + re
[PATCH v10 10/10] arch: sh: migor: Use new renesas-ceu camera driver
Migo-R platform uses sh_mobile_ceu camera driver, which is now being replaced by a proper V4L2 camera driver named 'renesas-ceu'. Move Migo-R platform to use the v4l2 renesas-ceu camera driver interface and get rid of soc_camera defined components used to register sensor drivers and of platform specific enable/disable routines. Register clock source and GPIOs for sensor drivers, so they can use clock and gpio APIs. Also, memory for CEU video buffers is now reserved with membocks APIs, and need to be declared as dma_coherent during machine initialization to remove that architecture specific part from CEU driver. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart Acked-by: Hans Verkuil --- arch/sh/boards/mach-migor/setup.c | 225 +++-- arch/sh/kernel/cpu/sh4a/clock-sh7722.c | 2 +- 2 files changed, 101 insertions(+), 126 deletions(-) diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c index 0bcbe58..271dfc2 100644 --- a/arch/sh/boards/mach-migor/setup.c +++ b/arch/sh/boards/mach-migor/setup.c @@ -1,17 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Renesas System Solutions Asia Pte. Ltd - Migo-R * * Copyright (C) 2008 Magnus Damm - * - * This file is subject to the terms and conditions of the GNU General Public - * License. See the file "COPYING" in the main directory of this archive - * for more details. */ +#include #include #include #include #include #include +#include #include #include #include @@ -23,10 +22,11 @@ #include #include #include +#include #include #include #include -#include +#include #include #include #include @@ -45,6 +45,9 @@ * 0x1800 8GB8 NAND Flash (K9K8G08U0A) */ +#define CEU_BUFFER_MEMORY_SIZE (4 << 20) +static phys_addr_t ceu_dma_membase; + static struct smc91x_platdata smc91x_info = { .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, }; @@ -301,65 +304,24 @@ static struct platform_device migor_lcdc_device = { }, }; -static struct clk *camera_clk; -static DEFINE_MUTEX(camera_lock); - -static void camera_power_on(int is_tw) -{ - mutex_lock(&camera_lock); - - /* Use 10 MHz VIO_CKO instead of 24 MHz to work -* around signal quality issues on Panel Board V2.1. -*/ - camera_clk = clk_get(NULL, "video_clk"); - clk_set_rate(camera_clk, 1000); - clk_enable(camera_clk); /* start VIO_CKO */ - - /* use VIO_RST to take camera out of reset */ - mdelay(10); - if (is_tw) { - gpio_set_value(GPIO_PTT2, 0); - gpio_set_value(GPIO_PTT0, 0); - } else { - gpio_set_value(GPIO_PTT0, 1); - } - gpio_set_value(GPIO_PTT3, 0); - mdelay(10); - gpio_set_value(GPIO_PTT3, 1); - mdelay(10); /* wait to let chip come out of reset */ -} - -static void camera_power_off(void) -{ - clk_disable(camera_clk); /* stop VIO_CKO */ - clk_put(camera_clk); - - gpio_set_value(GPIO_PTT3, 0); - mutex_unlock(&camera_lock); -} - -static int ov7725_power(struct device *dev, int mode) -{ - if (mode) - camera_power_on(0); - else - camera_power_off(); - - return 0; -} - -static int tw9910_power(struct device *dev, int mode) -{ - if (mode) - camera_power_on(1); - else - camera_power_off(); - - return 0; -} - -static struct sh_mobile_ceu_info sh_mobile_ceu_info = { - .flags = SH_CEU_FLAG_USE_8BIT_BUS, +static struct ceu_platform_data ceu_pdata = { + .num_subdevs= 2, + .subdevs = { + { /* [0] = ov772x */ + .flags = 0, + .bus_width = 8, + .bus_shift = 0, + .i2c_adapter_id = 0, + .i2c_address= 0x21, + }, + { /* [1] = tw9910 */ + .flags = 0, + .bus_width = 8, + .bus_shift = 0, + .i2c_adapter_id = 0, + .i2c_address= 0x45, + }, + }, }; static struct resource migor_ceu_resources[] = { @@ -373,18 +335,32 @@ static struct resource migor_ceu_resources[] = { .start = evt2irq(0x880), .flags = IORESOURCE_IRQ, }, - [2] = { - /* place holder for contiguous memory */ - }, }; static struct platform_device migor_ceu_device = { - .name = "sh_mobile_ceu", - .id = 0, /* "ceu0" clock */ + .name = "renesas-ceu", + .id = 0, /* ceu.0 */ .num_resources = ARRAY_SIZE(migor_ceu_resources), .resource = migor_ceu_resources, .dev= { - .platform_data = &sh_mobile_ceu_info, + .plat
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
On 02/21/2018 08:31 PM, Simon Horman wrote: > Populate the ethernet@e680 device node to enable Ethernet interface > for R-Car M3-N (r8a77965) SoC. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven > > --- > v1 -> v2: > - Replace ALWAYS_ON power area identifier with numeric constant > --- > arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 > ++- > 1 file changed, 42 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > index 55f05f7..c249895 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > @@ -520,7 +520,48 @@ > }; > > avb: ethernet@e680 { > - /* placeholder */ > + compatible = "renesas,etheravb-r8a77965", > + "renesas,etheravb-rcar-gen3"; > + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>; > + interrupts = , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + , > + ; > + interrupt-names = "ch0", "ch1", "ch2", "ch3", > + "ch4", "ch5", "ch6", "ch7", > + "ch8", "ch9", "ch10", "ch11", > + "ch12", "ch13", "ch14", "ch15", > + "ch16", "ch17", "ch18", "ch19", > + "ch20", "ch21", "ch22", "ch23", > + "ch24"; > + clocks = <&cpg CPG_MOD 812>; > + power-domains = <&sysc 32>; > + resets = <&cpg 812>; > + phy-mode = "rgmii-txid"; Why not just "rgmii"? TX delay is a board specific detail, no? >>> I admit I took this one straight from r8a7796 dtsi. >>> Would you like to me resend and change this? >> >>Yes, unless Simon would fix it while merging... > > Can I confirm the desired change is s/rgmii-txid/rgmii/ ? Yes. > If so I can fix that up. Thank you! MBR, Sergei
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
On Wed, Feb 21, 2018 at 06:48:59PM +0300, Sergei Shtylyov wrote: > Hello! > > On 02/21/2018 01:07 PM, jacopo mondi wrote: > > >>> Populate the ethernet@e680 device node to enable Ethernet interface > >>> for R-Car M3-N (r8a77965) SoC. > >>> > >>> Signed-off-by: Jacopo Mondi > >>> Reviewed-by: Geert Uytterhoeven > >>> > >>> --- > >>> v1 -> v2: > >>> - Replace ALWAYS_ON power area identifier with numeric constant > >>> --- > >>> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 > >>> ++- > >>> 1 file changed, 42 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > >>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > >>> index 55f05f7..c249895 100644 > >>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > >>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > >>> @@ -520,7 +520,48 @@ > >>> }; > >>> > >>> avb: ethernet@e680 { > >>> - /* placeholder */ > >>> + compatible = "renesas,etheravb-r8a77965", > >>> + "renesas,etheravb-rcar-gen3"; > >>> + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>; > >>> + interrupts = , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + , > >>> + ; > >>> + interrupt-names = "ch0", "ch1", "ch2", "ch3", > >>> + "ch4", "ch5", "ch6", "ch7", > >>> + "ch8", "ch9", "ch10", "ch11", > >>> + "ch12", "ch13", "ch14", "ch15", > >>> + "ch16", "ch17", "ch18", "ch19", > >>> + "ch20", "ch21", "ch22", "ch23", > >>> + "ch24"; > >>> + clocks = <&cpg CPG_MOD 812>; > >>> + power-domains = <&sysc 32>; > >>> + resets = <&cpg 812>; > >>> + phy-mode = "rgmii-txid"; > >> > >>Why not just "rgmii"? TX delay is a board specific detail, no? > >> > > > > I admit I took this one straight from r8a7796 dtsi. > > Would you like to me resend and change this? > >Yes, unless Simon would fix it while merging... Can I confirm the desired change is s/rgmii-txid/rgmii/ ? If so I can fix that up.
Re: [PATCH v2 15/19] dt-bindings: gpio: Add support for r8a77965
On Tue, Feb 20, 2018 at 04:12:17PM +0100, Jacopo Mondi wrote: > Add compatible string for R-Car M3-N (r8a77965) in gpio-rcar. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven Reviewed-by: Simon Horman
Re: [PATCH v2 16/19] ARM64: dts: r8a77965: Add GPIO nodes
On Tue, Feb 20, 2018 at 04:12:18PM +0100, Jacopo Mondi wrote: > Add GPIO nodes to r8a77965 SoC device tree file. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven > > --- > v1 -> v2: > - Replace ALWAYS_ON power area define with numeric constant > - Do not move gpio nodes Thanks, applied with prefix updated to: arm64: dts: renesas: r8a77965:
Re: [PATCH v2 14/19] ARM64: dts: r8a77965: Add SCIF device nodes
On Tue, Feb 20, 2018 at 04:12:16PM +0100, Jacopo Mondi wrote: > Add SCIF[0-5] device nodes for M3-N (r8a77965) SoC. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven Thanks, applied with prefix updated to: arm64: dts: renesas: r8a77965:
Re: [PATCH v2 11/19] ARM64: dts: r8a77965: Add dmac device nods
On Tue, Feb 20, 2018 at 04:12:13PM +0100, Jacopo Mondi wrote: > Add dmac[0-2] device nodes for R-Car M3-N (r8a77965) SoC. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven Thanks, applied with prefix updated to: arm64: dts: renesas: r8a77965:
Re: [PATCH v2 12/19] dt-bindings: serial: sh-sci: Add support for r8a77965 (H)SCIF
On Tue, Feb 20, 2018 at 04:12:14PM +0100, Jacopo Mondi wrote: > Add documentation for r8a77965 compatible string to Renesas sci-serial > device tree bindings documentation. > > Signed-off-by: Jacopo Mondi Reviewed-by: Simon Horman
Re: [PATCH v2 09/19] ARM64: dts: Add R-Car Salvator-x M3-N support
On Tue, Feb 20, 2018 at 04:46:23PM +0100, Geert Uytterhoeven wrote: > On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi > wrote: > > Add basic support for R-Car Salvator-X M3-N (R8A77965) board. > > > > Based on original work from: > > Takeshi Kihara > > Magnus Damm > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven Thanks, applied with prefix updated to "arm64: dts: renesas:" There was some fuzz applying this patch, the result is as follows. From: Jacopo Mondi Subject: [PATCH] arm64: dts: renesas: Add R-Car Salvator-x M3-N support Add basic support for R-Car Salvator-X M3-N (R8A77965) board. Based on original work from: Takeshi Kihara Magnus Damm Signed-off-by: Jacopo Mondi Reviewed-by: Geert Uytterhoeven --- arch/arm64/boot/dts/renesas/Makefile| 1 + arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts | 21 + 2 files changed, 22 insertions(+) create mode 100644 arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile index c885eef4e660..9ea5ec0daeba 100644 --- a/arch/arm64/boot/dts/renesas/Makefile +++ b/arch/arm64/boot/dts/renesas/Makefile @@ -7,6 +7,7 @@ dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-es1-h3ulcb-kf.dtb dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-x.dtb r8a7796-m3ulcb.dtb dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-m3ulcb-kf.dtb dtb-$(CONFIG_ARCH_R8A7796) += r8a7796-salvator-xs.dtb +dtb-$(CONFIG_ARCH_R8A77965) += r8a77965-salvator-x.dtb dtb-$(CONFIG_ARCH_R8A77970) += r8a77970-eagle.dtb r8a77970-v3msk.dtb dtb-$(CONFIG_ARCH_R8A77980) += r8a77980-condor.dtb dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb diff --git a/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts new file mode 100644 index ..75d890d91df9 --- /dev/null +++ b/arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Device Tree Source for the Salvator-X board with R-Car M3-N + * + * Copyright (C) 2018 Jacopo Mondi + */ + +/dts-v1/; +#include "r8a77965.dtsi" +#include "salvator-x.dtsi" + +/ { + model = "Renesas Salvator-X board based on r8a77965"; + compatible = "renesas,salvator-x", "renesas,r8a77965"; + + memory@4800 { + device_type = "memory"; + /* first 128MB is reserved for secure area. */ + reg = <0x0 0x4800 0x0 0x7800>; + }; +}; -- 2.11.0
Re: [PATCH v2 08/19] ARM64: dts: Add Renesas R8A77965 SoC support
On Tue, Feb 20, 2018 at 04:45:48PM +0100, Geert Uytterhoeven wrote: > On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi > wrote: > > Basic support for the Gen 3 R-Car M3-N SoC. > > > > Based on original work from: > > Takeshi Kihara > > Magnus Damm > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven Thanks, applied with the patch subject updated to arm64: dts: renesas: initial R8A77965 SoC device tree Please use "arm64: dts: renesas:" as the prefix for Gen-3 DTS/DTSI patches.
Re: [PATCH v2 07/19] ARM64: Add Renesas R-Car M3-N config symbol
On Tue, Feb 20, 2018 at 04:12:09PM +0100, Jacopo Mondi wrote: > Add configuration option for the R-Car M3-N (R8A77965) SoC. > > Signed-off-by: Jacopo Mondi Thanks, I have applied this after updating the subject to: arm64: add Renesas R8A77965 support Please use "arm64" rather than ARM64 as a prefix. (But "ARM" rather than "arm"!).
Re: [PATCH v2 06/19] dt-bindings: arm: Document R-Car M3-N SoC DT bindings
On Tue, Feb 20, 2018 at 04:12:08PM +0100, Jacopo Mondi wrote: > Add device tree bindings documentation for Renesas R-Car M3-N (r8a77965) > SoC. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Geert Uytterhoeven > Reviewed-by: Rob Herring Thanks, this is already applied.
Re: [PATCH v2 04/19] soc: renesas: rcar-sysc: Add R-Car M3-N support
On Tue, Feb 20, 2018 at 04:12:06PM +0100, Jacopo Mondi wrote: > Add support for R-Car M3-N (R8A77965) power areas. > > Signed-off-by: Jacopo Mondi > > --- > v1->v2: > - Remove A2VC0 power area > - Add A3VP power area > --- > .../bindings/power/renesas,rcar-sysc.txt | 1 + > drivers/soc/renesas/Kconfig| 5 +++ > drivers/soc/renesas/Makefile | 1 + > drivers/soc/renesas/r8a77965-sysc.c| 37 > ++ > drivers/soc/renesas/rcar-sysc.c| 3 ++ > drivers/soc/renesas/rcar-sysc.h| 1 + > include/dt-bindings/power/r8a77965-sysc.h | 30 ++ > 7 files changed, 78 insertions(+) > create mode 100644 drivers/soc/renesas/r8a77965-sysc.c > create mode 100644 include/dt-bindings/power/r8a77965-sysc.h Thanks, applied. There was some fuzz. The result is below. From: Jacopo Mondi Subject: [PATCH] soc: renesas: rcar-sysc: Add R-Car M3-N support Add support for R-Car M3-N (R8A77965) power areas. Signed-off-by: Jacopo Mondi Reviewed-by: Geert Uytterhoeven --- .../bindings/power/renesas,rcar-sysc.txt | 1 + drivers/soc/renesas/Kconfig| 5 +++ drivers/soc/renesas/Makefile | 1 + drivers/soc/renesas/r8a77965-sysc.c| 37 ++ drivers/soc/renesas/rcar-sysc.c| 3 ++ drivers/soc/renesas/rcar-sysc.h| 1 + include/dt-bindings/power/r8a77965-sysc.h | 30 ++ 7 files changed, 78 insertions(+) create mode 100644 drivers/soc/renesas/r8a77965-sysc.c create mode 100644 include/dt-bindings/power/r8a77965-sysc.h diff --git a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt index 6284a9550b3c..ab399e559257 100644 --- a/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt +++ b/Documentation/devicetree/bindings/power/renesas,rcar-sysc.txt @@ -17,6 +17,7 @@ Required properties: - "renesas,r8a7794-sysc" (R-Car E2) - "renesas,r8a7795-sysc" (R-Car H3) - "renesas,r8a7796-sysc" (R-Car M3-W) + - "renesas,r8a77965-sysc" (R-Car M3-N) - "renesas,r8a77970-sysc" (R-Car V3M) - "renesas,r8a77980-sysc" (R-Car V3H) - "renesas,r8a77995-sysc" (R-Car D3) diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig index 6caa393e3a2d..7106a6330210 100644 --- a/drivers/soc/renesas/Kconfig +++ b/drivers/soc/renesas/Kconfig @@ -14,6 +14,7 @@ config SOC_RENESAS select SYSC_R8A7794 if ARCH_R8A7794 select SYSC_R8A7795 if ARCH_R8A7795 select SYSC_R8A7796 if ARCH_R8A7796 + select SYSC_R8A77965 if ARCH_R8A77965 select SYSC_R8A77970 if ARCH_R8A77970 select SYSC_R8A77980 if ARCH_R8A77980 select SYSC_R8A77995 if ARCH_R8A77995 @@ -57,6 +58,10 @@ config SYSC_R8A7796 bool "R-Car M3-W System Controller support" if COMPILE_TEST select SYSC_RCAR +config SYSC_R8A77965 + bool "R-Car M3-N System Controller support" if COMPILE_TEST + select SYSC_RCAR + config SYSC_R8A77970 bool "R-Car V3M System Controller support" if COMPILE_TEST select SYSC_RCAR diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile index d3b7bb3284c0..ccb5ec57a262 100644 --- a/drivers/soc/renesas/Makefile +++ b/drivers/soc/renesas/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_SYSC_R8A7792)+= r8a7792-sysc.o obj-$(CONFIG_SYSC_R8A7794) += r8a7794-sysc.o obj-$(CONFIG_SYSC_R8A7795) += r8a7795-sysc.o obj-$(CONFIG_SYSC_R8A7796) += r8a7796-sysc.o +obj-$(CONFIG_SYSC_R8A77965)+= r8a77965-sysc.o obj-$(CONFIG_SYSC_R8A77970)+= r8a77970-sysc.o obj-$(CONFIG_SYSC_R8A77980)+= r8a77980-sysc.o obj-$(CONFIG_SYSC_R8A77995)+= r8a77995-sysc.o diff --git a/drivers/soc/renesas/r8a77965-sysc.c b/drivers/soc/renesas/r8a77965-sysc.c new file mode 100644 index ..d7f7928e3c07 --- /dev/null +++ b/drivers/soc/renesas/r8a77965-sysc.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Renesas R-Car M3-N System Controller + * Copyright (C) 2018 Jacopo Mondi + * + * Based on Renesas R-Car M3-W System Controller + * Copyright (C) 2016 Glider bvba + */ + +#include +#include + +#include + +#include "rcar-sysc.h" + +static const struct rcar_sysc_area r8a77965_areas[] __initconst = { + { "always-on", 0, 0, R8A77965_PD_ALWAYS_ON, -1, PD_ALWAYS_ON }, + { "ca57-scu", 0x1c0, 0, R8A77965_PD_CA57_SCU, R8A77965_PD_ALWAYS_ON, + PD_SCU }, + { "ca57-cpu0", 0x80, 0, R8A77965_PD_CA57_CPU0, R8A77965_PD_CA57_SCU, + PD_CPU_NOCR }, + { "ca57-cpu1", 0x80, 1, R8A77965_PD_CA57_CPU1, R8A77965_PD_CA57_SCU, + PD_CPU_NOCR }, + { "cr7",0x240, 0, R8A77965_PD_CR7, R8A77965_PD_ALWAYS_ON }, + { "a3vc", 0x380, 0, R8A77965_PD_A3VC, R8A77965_PD_ALWAYS_O
Re: [PATCH v2 03/19] soc: renesas: Identify R-Car M3-N
On Tue, Feb 20, 2018 at 04:40:34PM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi > wrote: > > Add support for indentifying R-Car M3-N (R8A77965) SoC. > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven > > > --- a/drivers/soc/renesas/renesas-soc.c > > +++ b/drivers/soc/renesas/renesas-soc.c > > @@ -149,6 +149,11 @@ static const struct renesas_soc soc_rcar_v3m > > __initconst __maybe_unused = { > > .id = 0x54, > > }; > > > > +static const struct renesas_soc soc_rcar_m3_n __initconst __maybe_unused = > > { > > + .family = &fam_rcar_gen3, > > + .id = 0x55, > > +}; > > Note that the list was sorted by SoC part number. Thanks, I have moved the above structure to between the similar structure for m3_w and v3m. The result is below. > > + > > static const struct renesas_soc soc_rcar_v3h __initconst __maybe_unused = { > > .family = &fam_rcar_gen3, > > .id = 0x56, > > @@ -214,6 +219,9 @@ static const struct of_device_id renesas_socs[] > > __initconst = { > > #ifdef CONFIG_ARCH_R8A7796 > > { .compatible = "renesas,r8a7796", .data = &soc_rcar_m3_w }, > > #endif > > +#ifdef CONFIG_ARCH_R8A77965 > > + { .compatible = "renesas,r8a77965", .data = &soc_rcar_m3_n }, > > +#endif > > #ifdef CONFIG_ARCH_R8A77970 > > { .compatible = "renesas,r8a77970", .data = &soc_rcar_v3m }, > > #endif From: Jacopo Mondi Subject: [PATCH] soc: renesas: Identify R-Car M3-N Add support for indentifying R-Car M3-N (R8A77965) SoC. Signed-off-by: Jacopo Mondi Reviewed-by: Geert Uytterhoeven Signed-off-by: Simon Horman --- drivers/soc/renesas/renesas-soc.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c index 000834321774..ea71c413c926 100644 --- a/drivers/soc/renesas/renesas-soc.c +++ b/drivers/soc/renesas/renesas-soc.c @@ -144,6 +144,11 @@ static const struct renesas_soc soc_rcar_m3_w __initconst __maybe_unused = { .id = 0x52, }; +static const struct renesas_soc soc_rcar_m3_n __initconst __maybe_unused = { + .family = &fam_rcar_gen3, + .id = 0x55, +}; + static const struct renesas_soc soc_rcar_v3m __initconst __maybe_unused = { .family = &fam_rcar_gen3, .id = 0x54, @@ -214,6 +219,9 @@ static const struct of_device_id renesas_socs[] __initconst = { #ifdef CONFIG_ARCH_R8A7796 { .compatible = "renesas,r8a7796", .data = &soc_rcar_m3_w }, #endif +#ifdef CONFIG_ARCH_R8A77965 + { .compatible = "renesas,r8a77965", .data = &soc_rcar_m3_n }, +#endif #ifdef CONFIG_ARCH_R8A77970 { .compatible = "renesas,r8a77970", .data = &soc_rcar_v3m }, #endif -- 2.11.0
Re: [PATCH 2/2] vfio: platform: Add generic DT reset support
Hi Geert, I have a suggestion to avoid having to use the IS_ERR_OR_NULL macro, see below: On Tue, 2018-02-13 at 17:36 +0100, Geert Uytterhoeven wrote: > Vfio-platform requires reset support, provided either by ACPI, or, on DT > platforms, by a device-specific reset driver matching against the > device's compatible value. > > On many SoCs, devices are connected to an SoC-internal reset controller, > and can be reset in a generic way. Hence add support to reset such > devices using the reset controller subsystem, provided the reset > hierarchy is described correctly in DT using the "resets" property. > > Devices that require a more complex reset procedure can still > provide a device-specific reset driver, as that takes precedence. > > Note that this functionality depends on CONFIG_RESET_CONTROLLER=y, and > becomes a no-op if reset controller support is disabled. > > Signed-off-by: Geert Uytterhoeven > --- > drivers/vfio/platform/vfio_platform_common.c | 23 +-- > drivers/vfio/platform/vfio_platform_private.h | 1 + > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform_common.c > b/drivers/vfio/platform/vfio_platform_common.c > index b60bb5326668498c..5d1e48f96e423508 100644 > --- a/drivers/vfio/platform/vfio_platform_common.c > +++ b/drivers/vfio/platform/vfio_platform_common.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -112,7 +113,13 @@ static bool vfio_platform_has_reset(struct > vfio_platform_device *vdev) > if (VFIO_PLATFORM_IS_ACPI(vdev)) > return vfio_platform_acpi_has_reset(vdev); > > - return vdev->of_reset ? true : false; > + if (vdev->of_reset) > + return true; > + > + if (!IS_ERR_OR_NULL(vdev->reset_control)) > + return true; I'd avoid storing error values in vdev->reset_control at all, so this could be: if (vdev->reset_control) return true; > + > + return false; > } > > static int vfio_platform_get_reset(struct vfio_platform_device *vdev) > @@ -127,8 +134,15 @@ static int vfio_platform_get_reset(struct > vfio_platform_device *vdev) > vdev->of_reset = vfio_platform_lookup_reset(vdev->compat, > &vdev->reset_module); > } > + if (vdev->of_reset) > + return 0; > + > + vdev->reset_control = __of_reset_control_get(vdev->device->of_node, > + NULL, 0, false, false); > + if (!IS_ERR(vdev->reset_control)) > + return 0; if you assign to a local variable first here: struct reset_control *rstc; ... rstc = of_reset_control_get_exclusive(vdev->device->of_node, NULL); if (!IS_ERR(rstc)) { vdev->reset_control = rstc; return 0; } Also, please don't use __of_reset_control_get directly. > > - return vdev->of_reset ? 0 : -ENOENT; > + return PTR_ERR(vdev->reset_control); return PTR_ERR(rstc); > } > > static void vfio_platform_put_reset(struct vfio_platform_device *vdev) > @@ -138,6 +152,8 @@ static void vfio_platform_put_reset(struct > vfio_platform_device *vdev) > > if (vdev->of_reset) > module_put(vdev->reset_module); > + > + reset_control_put(vdev->reset_control); > } > > static int vfio_platform_regions_init(struct vfio_platform_device *vdev) > @@ -217,6 +233,9 @@ static int vfio_platform_call_reset(struct > vfio_platform_device *vdev, > } else if (vdev->of_reset) { > dev_info(vdev->device, "reset\n"); > return vdev->of_reset(vdev); > + } else if (!IS_ERR_OR_NULL(vdev->reset_control)) { > + dev_info(vdev->device, "reset\n"); > + return reset_control_reset(vdev->reset_control); } else { if (vdev->reset_control) dev_info(vdev->device, "reset\n"); return reset_control_reset(vdev->reset_control); > } > > dev_warn(vdev->device, "no reset function found!\n"); > diff --git a/drivers/vfio/platform/vfio_platform_private.h > b/drivers/vfio/platform/vfio_platform_private.h > index 85ffe5d9d1abd94e..a56e80ae5986540b 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -60,6 +60,7 @@ struct vfio_platform_device { > const char *compat; > const char *acpihid; > struct module *reset_module; > + struct reset_control*reset_control; > struct device *device; > > /* regards Philipp
Re: [PATCH v2 02/19] soc: renesas: rcar-rst: Add support for R-Car M3-N
On Tue, Feb 20, 2018 at 04:29:37PM +0100, Geert Uytterhoeven wrote: > On Tue, Feb 20, 2018 at 4:12 PM, Jacopo Mondi > wrote: > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven Thanks, applied.
Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver
Hi Laurent, Hans, On Wed, Feb 21, 2018 at 02:02:59PM +0100, Hans Verkuil wrote: > On 02/21/18 13:29, Laurent Pinchart wrote: > > Hi Hans, > > > > On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote: > >> On 02/19/18 17:59, Jacopo Mondi wrote: > >>> Add driver for Renesas Capture Engine Unit (CEU). > >>> > >>> The CEU interface supports capturing 'data' (YUV422) and 'images' > >>> (NV[12|21|16|61]). > >>> > >>> This driver aims to replace the soc_camera-based sh_mobile_ceu one. > >>> > >>> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ > >>> platform GR-Peach. > >>> > >>> Tested with ov7725 camera sensor on SH4 platform Migo-R. > >>> > >>> Signed-off-by: Jacopo Mondi > >>> Reviewed-by: Laurent Pinchart > >>> --- > >>> > >>> drivers/media/platform/Kconfig |9 + > >>> drivers/media/platform/Makefile |1 + > >>> drivers/media/platform/renesas-ceu.c | 1661 + > >>> 3 files changed, 1671 insertions(+) > >>> create mode 100644 drivers/media/platform/renesas-ceu.c > >> > >> > >> > >>> +static int ceu_s_input(struct file *file, void *priv, unsigned int i) > >>> +{ > >>> + struct ceu_device *ceudev = video_drvdata(file); > >>> + struct ceu_subdev *ceu_sd_old; > >>> + int ret; > >>> + > >>> + if (i >= ceudev->num_sd) > >>> + return -EINVAL; > >>> + > >>> + if (vb2_is_streaming(&ceudev->vb2_vq)) > >>> + return -EBUSY; > >>> + > >>> + if (i == ceudev->sd_index) > >>> + return 0; > >>> + > >>> + ceu_sd_old = ceudev->sd; > >>> + ceudev->sd = &ceudev->subdevs[i]; > >>> + > >>> + /* Make sure we can generate output image formats. */ > >>> + ret = ceu_init_formats(ceudev); > >> > >> Why is this done for every s_input? I would expect that this is done only > >> once for each subdev. > >> > >> I also expect to see a ceu_set_default_fmt() call here. Or that the > >> v4l2_pix > >> is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev. > >> I think I prefer that over configuring a new default format every time you > >> switch inputs. > > > > What does userspace expect today ? I don't think we document anywhere that > > formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is > > very vague: > > > > "To select a video input applications store the number of the desired input > > in > > an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer. > > Side effects are possible. For example inputs may support different video > > standards, so the driver may implicitly switch the current standard. Because > > of these possible side effects applications must select an input before > > querying or negotiating any other parameters." > > > > I understand that as meaning "anything can happen when you switch inputs, so > > be prepared to reconfigure everything explicitly without assuming any > > particular parameter to have a sane value". > > > >> This code will work for two subdevs with exactly the same > >> formats/properties. But switching between e.g. a sensor and a video > >> receiver will leave things in an inconsistent state as far as I can see. > >> > >> E.g. if input 1 is the video receiver then switching to that input and > >> running 'v4l2-ctl -V' will show the sensor format, not the video receiver > >> format. > > > > I agree that the format should be consistent with the selected input, as > > calling VIDIOC_S_INPUT immediately followed by a stream start sequence > > without > > configuring formats should work (but produce a format that is not known to > > userspace). My question is whether we should reset to a default format for > > the > > newly select input, or switch back to the previously set format. I'd tend to > > go for the former to keep as little state as possible, but I'm open to > > counter-proposals. > > What to do is up to the driver. My personal preference is to make it > persistent > per input, but that's just me. I won't block the other approach (resetting it > to a default). Be aware that for video receivers the format depends on the > current > selected standard as well. I think the code does that correctly already, but > it > would be good to verify if possible. > > BTW, I think it is right that the spec isn't specific about what changes when > you switch inputs. It can be quite complex for drivers to handle this and it > is not unreasonable in my view for userspace to just assume it needs to > configure > from scratch after switching inputs. > Given that there are not strict requisites here I will go for the default format selection. While there I'll rename the ceu_init_formats() function to ceu_init_mbus_fmt() as that's what it actually does, and move ceudev->field initialization from there to ceu_set[_default]_fmt() functions. I'm sending v10 with this changes hopefully quite soon. Thanks j > Regards, > > Hans > > > > >>> + if (ret) { > >>> + ceudev->sd = ceu_sd_old; > >>> + return -EINVAL; > >>> + }
Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
On Wed, Feb 21, 2018 at 01:10:30AM +0200, Laurent Pinchart wrote: > Hello, > > This patch series addresses a design mistake that dates back from the initial > DU support. Support for the LVDS encoders, which are IP cores separate from > the DU, was bundled in the DU driver. Worse, both the DU and LVDS were > described through a single DT node. > > To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS > encoders, and deprecate their description inside the DU bindings. To retain > backward compatibility with existing DT, patches 03/16 to 08/16 then patch the > device tree at runtime to convert the legacy bindings to the new ones. > > With the DT side addressed, patch 09/16 converts the LVDS support code to a > separate bridge driver. Patches 11/16 to 16/16 then update all the device tree > sources to the new DU and LVDS encoders bindings. > > I decided to go for live DT patching in patch 08/16 because implementing > support for both the legacy and new bindings in the driver would have been > very intrusive, and prevented further cleanups. This version relies more > heavily on overlays to avoid touching the internals of the OF core compared to > v2, even if manual fixes to the device tree are still needed. > > Compared to v3, this series uses the OF changeset API to update properties > instead of accessing the internals of the property structure. This removes the > local implementation of functions to look up nodes by path and update > properties. In order to do this, I pulled in Pantelis' patch series titled > "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and > rebased it while taking two small review comments into account. > > Rob, I'd like this series to be merged in v4.17. As the changeset helpers are > now a dependency, I'd need you to merge them early (ideally on top of > v4.16-rc1) and provide a stable branch, or get your ack to merge them through > Dave's tree if they don't conflict with what you have and will queue for > v4.17. > > This version also drops the small fix to the Porter board device tree that has > been queued for v4.17 already. > > Compared to v2, the biggest change is in patch 03/16. Following Rob's and > Frank's reviews it was clear that modifying the unflattened DT structure of > the overlay before applying it wasn't popular. I have thus decided to use one > overlay source per SoC to move as much of the DT changes to the overlay as > possible, and only perform manual modifications (that are still needed as some > of the information is board-specific) on the system DT after applying the > overlay. As a result the overlay is parsed and applied without being modified. > > Compared to v1, this series update the r8a7792 and r8a7794 device tree sources > and incorporate review feedback as described by the changelogs of individual > patches. > > > Laurent Pinchart (11): > dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings > dt-bindings: display: renesas: Deprecate LVDS support in the DU > bindings > drm: rcar-du: Fix legacy DT to create LVDS encoder nodes > drm: rcar-du: Convert LVDS encoder code to bridge driver > ARM: dts: r8a7790: Convert to new LVDS DT bindings > ARM: dts: r8a7791: Convert to new LVDS DT bindings > ARM: dts: r8a7792: Convert to new DU DT bindings > ARM: dts: r8a7793: Convert to new LVDS DT bindings > ARM: dts: r8a7794: Convert to new DU DT bindings > arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings > arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings I have marked the dts patches above as deferred as they depend on the driver changes not to cause a regression. Please repost them or otherwise ping me once the driver dependencies are present in an rc release. I am assuming that the other patches in this series are not targeted at the renesas tree. > > Pantelis Antoniou (5): > of: dynamic: Add __of_node_dupv() > of: changesets: Introduce changeset helper methods > of: changeset: Add of_changeset_node_move method > of: unittest: changeset helpers > i2c: demux: Use changeset helpers for clarity ...
Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1
Hi Simon, On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman wrote: > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote: >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro >> wrote: >> > this series has been around for some time as RFC, and it has collected >> > useful comments from the community along the way. >> > The solution proposed by this patch set works for most R-Car Gen2 and >> > RZ/G1 devices, but not all of them. We now know that for some R-Car >> > Gen2 early revisions there is no proper software fix. Anyway, no >> > product has been built around early revisions, but development boards >> > mounting early revisions (basically prototypes) are still out there. >> > As a result, this series isn't enabling the internal watchdog on R-Car >> > Gen2 boards, developers may enable it in board specific device trees >> > if needed. >> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt, >> > and Koelsch boards. >> > >> > The problem >> > === >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector >> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON >> > the non-boot CPUs they are redirected to the reset vector installed by >> > Linux in ICRAM1, and eventually they continue the execution to RAM, >> > where the SMP bring-up code will take care of the rest. >> > The content of the [S]BAR registers survives a watchdog triggered reset, >> > and as such after the watchdog fires the boot core will try and execute >> > the SMP bring-up code instead of jumping to the bootrom code. >> > >> > The fix >> > === >> > The main strategy for the solution is to let the reset vector decide >> > if it needs to jump to shmobile_boot_fn or to the bootrom code. >> > In a watchdog triggered reset scenario, since the [S]BAR registers keep >> > their values, the boot CPU will jump into the newly designed reset >> > vector, the assembly routine will eventually test WOVF (a bit in register >> > RWTCSRA that indicates if the watchdog counter has overflown, the value >> > of this bit gets retained in this scenario), and jump to the bootrom code >> > which will in turn load up the bootloader, etc. >> > When bringing up SMP or using CPU hotplug, the reset vector will jump >> > to shmobile_boot_fn instead. >> > >> > Thank you All for your help. >> > >> > Best regards, >> > >> > Fabrizio Castro (26): >> > ARM: shmobile: Add watchdog support >> > ARM: dts: r8a7743: Adjust SMP routine size >> > ARM: dts: r8a7745: Adjust SMP routine size >> > ARM: dts: r8a7790: Adjust SMP routine size >> > ARM: dts: r8a7791: Adjust SMP routine size >> > ARM: dts: r8a7792: Adjust SMP routine size >> > ARM: dts: r8a7793: Adjust SMP routine size >> > ARM: dts: r8a7794: Adjust SMP routine size >> > soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2 >> > ARM: shmobile: rcar-gen2: Add watchdog support >> > dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support >> > watchdog: renesas_wdt: Add R-Car Gen2 support >> > watchdog: renesas_wdt: Add restart handler >> > ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN >> > clk: renesas: r8a7743: Add rwdt clock >> > clk: renesas: r8a7745: Add rwdt clock >> > clk: renesas: r8a7790: Add rwdt clock >> > clk: renesas: r8a7791/r8a7793: Add rwdt clock >> > clk: renesas: r8a7794: Add rwdt clock >> > ARM: dts: r8a7743: Add watchdog support to SoC dtsi >> > ARM: dts: r8a7745: Add watchdog support to SoC dtsi >> > ARM: dts: r8a7790: Add watchdog support to SoC dtsi >> > ARM: dts: r8a7791: Add watchdog support to SoC dtsi >> > ARM: dts: r8a7794: Add watchdog support to SoC dtsi >> > ARM: dts: iwg20m: Add watchdog support to SoM dtsi >> > ARM: dts: iwg22m: Add watchdog support to SoM dtsi >> > >> > .../devicetree/bindings/watchdog/renesas-wdt.txt | 19 ++-- >> > arch/arm/boot/dts/r8a7743-iwg20m.dtsi | 5 ++ >> > arch/arm/boot/dts/r8a7743.dtsi | 12 - >> > arch/arm/boot/dts/r8a7745-iwg22m.dtsi | 5 ++ >> > arch/arm/boot/dts/r8a7745.dtsi | 12 - >> > arch/arm/boot/dts/r8a7790.dtsi | 12 - >> > arch/arm/boot/dts/r8a7791.dtsi | 12 - >> > arch/arm/boot/dts/r8a7792.dtsi | 2 +- >> > arch/arm/boot/dts/r8a7793.dtsi | 2 +- >> > arch/arm/boot/dts/r8a7794.dtsi | 12 - >> > arch/arm/configs/shmobile_defconfig| 1 + >> > arch/arm/mach-shmobile/common.h| 6 +++ >> > arch/arm/mach-shmobile/headsmp.S | 55 >> > ++ >> > arch/arm/mach-shmobile/platsmp-apmu.c | 1 + >> > arch/arm/mach-shmobile/pm-rcar-gen2.c | 15 -- >> > drivers/clk/renesas/r8a7743-cpg-mssr.c | 2 + >> > drivers/clk/renesas/r8a7745-cpg-mssr.c | 2 + >> > drivers/clk/renesas/r8a7790-cpg-mssr.c
Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt
On 02/21/2018 04:47 PM, jacopo mondi wrote: > Hello again, > > On Tue, Feb 20, 2018 at 09:58:57AM +0100, jacopo mondi wrote: >> Hi Laurent, >> >> On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote: >>> Hi Jacopo, >>> >>> Thank you for the patch. >>> >>> On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote: The sensor driver sets mbus format colorspace information and sizes, but not ycbcr encoding, quantization and xfer function. When supplied with an badly initialized mbus frame format structure, those fields need to be set explicitly not to leave them uninitialized. This is tested by v4l2-compliance, which supplies a mbus format description structure and checks for all fields to be properly set. Without this commit, v4l2-compliance fails when testing formats with: fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff Signed-off-by: Jacopo Mondi --- drivers/media/i2c/ov7670.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c index 25b26d4..61c472e 100644 --- a/drivers/media/i2c/ov7670.c +++ b/drivers/media/i2c/ov7670.c @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd, fmt->height = wsize->height; fmt->colorspace = ov7670_formats[index].colorspace; >>> >>> On a side note, if I'm not mistaken the colorspace field is set to SRGB for >>> all entries. Shouldn't you hardcode it here and remove the field ? >>> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; >>> >>> How about setting the values explicitly instead of relying on defaults ? >>> That >>> would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and >>> V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if the >>> sensor outputs limited or full range ? >>> >> >> This actually makes me wonder if those informations (ycbcb_enc, >> quantization and xfer_func) shouldn't actually be part of the >> supported format list... I blindly added those default fields in the >> try_fmt function, but I doubt they applies to all supported formats. >> >> Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and >> RGB 565) and 1 raw format (BGGR). I now have a question here: >> >> 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this >> applies to RGB and raw formats? I don't think so, and what value is >> the correct one for the ycbcr_enc field in this case? I assume >> xfer_func and quantization applies to all formats instead.. >> > > What if I repropose this as separate patch not part of the CEU series > in order not to hold back v10 (which I hope will be the last CEU > iteration)? I would definitely be fine with that. We first need to define what exactly should be done by drivers. See also this thread: https://www.spinics.net/lists/linux-renesas-soc/msg23888.html I'll work on that going forward as part of the compliance tests. Regards, Hans > >> Thanks >>j >> info->format = *fmt; return 0; >>> >>> -- >>> Regards, >>> >>> Laurent Pinchart >>>
Re: [PATCH] watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs
On Wed, Feb 21, 2018 at 04:43:04PM +0100, Geert Uytterhoeven wrote: > On early revisions of some R-Car Gen2 SoCs, and depending on SMP > configuration, the system may fail to restart on watchdog time-out, and > lock up instead. > > Specifically: > - On R-Car H2 ES1.0 and M2-W ES1.0, watchdog restart fails unless > only the first CPU core is in use (using e.g. the "maxcpus=1" kernel > commandline option). > - On R-Car V2H ES1.1, watchdog restart fails unless SMP is disabled > completely (using CONFIG_SMP=n during build configuration, or using > the "nosmp" or "maxcpus=0" kernel commandline options). > > Prevent using the watchdog in impacted cases by blacklisting the > affected SoCs, using the minimum known working revisions (ES2.0 on R-Car > H2, and ES3.0 on M2-W), and taking the actual SMP software configuration > into account. > > Signed-off-by: Geert Uytterhoeven > --- > To be folded into Fabrizio Castro's "watchdog: renesas_wdt: Add R-Car > Gen2 support". > > Note that I cannot use IS_ENABLED(CONFIG_SMP), as setup_max_cpus does > not exist for CONFIG_SMP=n. Thanks, I was going to ask about that. Reviewed-by: Simon Horman > Any reports on R-Car M2-W ES2.0 and V2H ES2.0+ are welcomed! > > As the failure is due to an integration issue, and the watchdog itself > is working fine, an alternative solution would be to move the check to > the code that installs the reset trigger ("soc: renesas: rcar-rst: > Enable watchdog as reset trigger for Gen2"). > However, doing so would mean that: > 1. The user could enable and seemingly use the watchdog, but watchdog > timeout would not restart the system, > 2. The same check should be done before installing the new reset > vector ("ARM: shmobile: rcar-gen2: Add watchdog support"), too, > else onlining CPU0 would fail once the watchdog has timed out. > --- > drivers/watchdog/renesas_wdt.c | 43 > ++ > 1 file changed, 43 insertions(+) > > diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c > index 0f88614797c36022..4c8e8d2600a922a5 100644 > --- a/drivers/watchdog/renesas_wdt.c > +++ b/drivers/watchdog/renesas_wdt.c > @@ -16,6 +16,8 @@ > #include > #include > #include > +#include > +#include > #include > > #define RWTCNT 0 > @@ -131,6 +133,44 @@ static const struct watchdog_ops rwdt_ops = { > .restart = rwdt_restart, > }; > > +#if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP) > +/* > + * Watchdog-reset integration is broken on early revisions of R-Car Gen2 SoCs > + */ > +static const struct soc_device_attribute rwdt_quirks_match[] = { > + { > + .soc_id = "r8a7790", > + .revision = "ES1.*", > + .data = (void *)1, /* needs single CPU */ > + }, { > + .soc_id = "r8a7791", > + .revision = "ES[12].*", > + .data = (void *)1, /* needs single CPU */ > + }, { > + .soc_id = "r8a7792", > + .revision = "*", > + .data = (void *)0, /* needs SMP disabled */ > + }, > + { /* sentinel */ } > +}; > + > +static bool rwdt_blacklisted(struct device *dev) > +{ > + const struct soc_device_attribute *attr; > + > + attr = soc_device_match(rwdt_quirks_match); > + if (attr && setup_max_cpus > (uintptr_t)attr->data) { > + dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id, > + attr->revision); > + return true; > + } > + > + return false; > +} > +#else /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */ > +static inline bool rwdt_blacklisted(struct device *dev) { return false; } > +#endif /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */ > + > static int rwdt_probe(struct platform_device *pdev) > { > struct rwdt_priv *priv; > @@ -139,6 +179,9 @@ static int rwdt_probe(struct platform_device *pdev) > unsigned long clks_per_sec; > int ret, i; > > + if (rwdt_blacklisted(&pdev->dev)) > + return -ENODEV; > + > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > if (!priv) > return -ENOMEM; > -- > 2.7.4 >
Re: [PATCH v2] arm64: defconfig: Enable PWM and USB for R-Car
On Wed, Feb 21, 2018 at 02:57:16PM +0900, Yoshihiro Shimoda wrote: > Enables PWM controller, USB-DMAC that is used by HS-USB, USB 3.0 > peripheral controller and USB 3.0 PHY for R-Car SoCs. > > Signed-off-by: Yoshihiro Shimoda > Acked-by: Geert Uytterhoeven Thanks, applied.
Re: [PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1
On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote: > On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro > wrote: > > this series has been around for some time as RFC, and it has collected > > useful comments from the community along the way. > > The solution proposed by this patch set works for most R-Car Gen2 and > > RZ/G1 devices, but not all of them. We now know that for some R-Car > > Gen2 early revisions there is no proper software fix. Anyway, no > > product has been built around early revisions, but development boards > > mounting early revisions (basically prototypes) are still out there. > > As a result, this series isn't enabling the internal watchdog on R-Car > > Gen2 boards, developers may enable it in board specific device trees > > if needed. > > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt, > > and Koelsch boards. > > > > The problem > > === > > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector > > to ICRAM1 and we program the [S]BAR registers so that when we turn ON > > the non-boot CPUs they are redirected to the reset vector installed by > > Linux in ICRAM1, and eventually they continue the execution to RAM, > > where the SMP bring-up code will take care of the rest. > > The content of the [S]BAR registers survives a watchdog triggered reset, > > and as such after the watchdog fires the boot core will try and execute > > the SMP bring-up code instead of jumping to the bootrom code. > > > > The fix > > === > > The main strategy for the solution is to let the reset vector decide > > if it needs to jump to shmobile_boot_fn or to the bootrom code. > > In a watchdog triggered reset scenario, since the [S]BAR registers keep > > their values, the boot CPU will jump into the newly designed reset > > vector, the assembly routine will eventually test WOVF (a bit in register > > RWTCSRA that indicates if the watchdog counter has overflown, the value > > of this bit gets retained in this scenario), and jump to the bootrom code > > which will in turn load up the bootloader, etc. > > When bringing up SMP or using CPU hotplug, the reset vector will jump > > to shmobile_boot_fn instead. > > > > Thank you All for your help. > > > > Best regards, > > > > Fabrizio Castro (26): > > ARM: shmobile: Add watchdog support > > ARM: dts: r8a7743: Adjust SMP routine size > > ARM: dts: r8a7745: Adjust SMP routine size > > ARM: dts: r8a7790: Adjust SMP routine size > > ARM: dts: r8a7791: Adjust SMP routine size > > ARM: dts: r8a7792: Adjust SMP routine size > > ARM: dts: r8a7793: Adjust SMP routine size > > ARM: dts: r8a7794: Adjust SMP routine size > > soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2 > > ARM: shmobile: rcar-gen2: Add watchdog support > > dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support > > watchdog: renesas_wdt: Add R-Car Gen2 support > > watchdog: renesas_wdt: Add restart handler > > ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN > > clk: renesas: r8a7743: Add rwdt clock > > clk: renesas: r8a7745: Add rwdt clock > > clk: renesas: r8a7790: Add rwdt clock > > clk: renesas: r8a7791/r8a7793: Add rwdt clock > > clk: renesas: r8a7794: Add rwdt clock > > ARM: dts: r8a7743: Add watchdog support to SoC dtsi > > ARM: dts: r8a7745: Add watchdog support to SoC dtsi > > ARM: dts: r8a7790: Add watchdog support to SoC dtsi > > ARM: dts: r8a7791: Add watchdog support to SoC dtsi > > ARM: dts: r8a7794: Add watchdog support to SoC dtsi > > ARM: dts: iwg20m: Add watchdog support to SoM dtsi > > ARM: dts: iwg22m: Add watchdog support to SoM dtsi > > > > .../devicetree/bindings/watchdog/renesas-wdt.txt | 19 ++-- > > arch/arm/boot/dts/r8a7743-iwg20m.dtsi | 5 ++ > > arch/arm/boot/dts/r8a7743.dtsi | 12 - > > arch/arm/boot/dts/r8a7745-iwg22m.dtsi | 5 ++ > > arch/arm/boot/dts/r8a7745.dtsi | 12 - > > arch/arm/boot/dts/r8a7790.dtsi | 12 - > > arch/arm/boot/dts/r8a7791.dtsi | 12 - > > arch/arm/boot/dts/r8a7792.dtsi | 2 +- > > arch/arm/boot/dts/r8a7793.dtsi | 2 +- > > arch/arm/boot/dts/r8a7794.dtsi | 12 - > > arch/arm/configs/shmobile_defconfig| 1 + > > arch/arm/mach-shmobile/common.h| 6 +++ > > arch/arm/mach-shmobile/headsmp.S | 55 > > ++ > > arch/arm/mach-shmobile/platsmp-apmu.c | 1 + > > arch/arm/mach-shmobile/pm-rcar-gen2.c | 15 -- > > drivers/clk/renesas/r8a7743-cpg-mssr.c | 2 + > > drivers/clk/renesas/r8a7745-cpg-mssr.c | 2 + > > drivers/clk/renesas/r8a7790-cpg-mssr.c | 2 + > > drivers/clk/renesas/r8a7791-cpg-mssr.c | 2 + > > drivers/clk/renesas/r8a7794-cpg-mssr.c | 2 + > > drivers/soc/renesas
Re: [PATCH 2/2] vfio: platform: Add generic DT reset support
Hi Eric, On Wed, Feb 14, 2018 at 11:11 AM, Auger Eric wrote: > On 14/02/18 10:43, Geert Uytterhoeven wrote: >> On Wed, Feb 14, 2018 at 10:09 AM, Auger Eric wrote: >>> On 13/02/18 17:36, Geert Uytterhoeven wrote: Vfio-platform requires reset support, provided either by ACPI, or, on DT platforms, by a device-specific reset driver matching against the device's compatible value. On many SoCs, devices are connected to an SoC-internal reset controller, and can be reset in a generic way. Hence add support to reset such devices using the reset controller subsystem, provided the reset hierarchy is described correctly in DT using the "resets" property. >>> >>> I first acknowledge I am not familiar with what those reset controllers >>> do in practice. My fear is that we may rely on generic SW pieces that >>> may not be adapted to passthrough constraints. We must guarantee that >>> any DMA access attempted by the devices are stopped and any interrupts >>> gets stopped. Can we guarantee that the reset controller always induce >>> that? Otherwise we may leave the door opened to badly reset assigned >>> devices. >> >> An on-SoC reset controller is basically a block controlling signals to the >> reset inputs of the individual on-SoC devices. >> On Renesas ARM SoCs, this allows to do a full reset of the attached device. >> >> Of course the exact semantics depend on the actual SoC. > that's the issue actually. >> If e.g. DMA and interrupts are not stopped for a specific device on a >> specific SoC, it still needs a device-specific reset driver, matching against >> the appropriate compatible value, cfr. the quoted paragraph below. > yes but by default we still accept the reset controller solution. >> >> You could add a whitelist for of_machine_is_compatible() or >> of_device_is_compatible(), but that will grow large fast. > Could be the kind of solution needed. At the moment the list of assigned > platform devices is pretty reduced. > > Couldn't we imagine additional dt properties to emphasize the fact a > platform device is passthrough friendly in terms of reset, either > through a reset controller or exposing a single reg that need to be > reset for full reset to be achieved, in accordance with assignment > constraints. That way, the driver writer would somehow certify the > device is eligible for passthrough. One of the issue today is that the > vfio platform reset driver is not maintained by the native driver > maintainer. I'm not so fond of adding more DT properties for this. They can be abused as well. In general, if there's a "resets" DT property, it means the device can be reset through the pointed-by reset controller. So that's the common case, which I'd like to optimize/simplify for. If more is needed, a separate (device-specific) vfio_reset handler needs to be written, by the people that know the hardware. > I think if people want to do platform passthrough, they need to devise > their HW IPs so that this reset modality is simplified by exposing this > kind of single reg and then dt description may expose this. Also if > possible, the dt node must be as simple/generic as possible to avoid > writing a huge dt node creation function on QEMU side and avoid > dependencies on other nodes. Yes. It all depends on sane hardware ;-) 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 1/2] vfio: platform: Fix reset module leak in error path
Hi Eric, On Wed, Feb 14, 2018 at 10:32 AM, Geert Uytterhoeven wrote: > On Wed, Feb 14, 2018 at 9:36 AM, Auger Eric wrote: >> If I am not wrong we also leak the reset_module if >> vfio_platform_get_reset() fails to find the reset function (of_reset == >> NULL), in which case we should do the module_put() in >> vfio_platform_get_reset(). > > Correct. Will look into fixing it... Upon second look, I don't think there's a leak in vfio_platform_get_reset(). If try_module_get() succeeded, there will always be a valid reset function (unless someone registered a vfio_reset_handler with a NULL reset function). Or do you mean the call to request_module()? That one doesn't do a module_get(), it merely tries to load a module. Hence there's no need to do a module_put() afterwards. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
Hello! On 02/21/2018 01:07 PM, jacopo mondi wrote: >>> Populate the ethernet@e680 device node to enable Ethernet interface >>> for R-Car M3-N (r8a77965) SoC. >>> >>> Signed-off-by: Jacopo Mondi >>> Reviewed-by: Geert Uytterhoeven >>> >>> --- >>> v1 -> v2: >>> - Replace ALWAYS_ON power area identifier with numeric constant >>> --- >>> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 >>> ++- >>> 1 file changed, 42 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi >>> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >>> index 55f05f7..c249895 100644 >>> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi >>> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >>> @@ -520,7 +520,48 @@ >>> }; >>> >>> avb: ethernet@e680 { >>> - /* placeholder */ >>> + compatible = "renesas,etheravb-r8a77965", >>> +"renesas,etheravb-rcar-gen3"; >>> + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>; >>> + interrupts = , >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +, >>> +; >>> + interrupt-names = "ch0", "ch1", "ch2", "ch3", >>> + "ch4", "ch5", "ch6", "ch7", >>> + "ch8", "ch9", "ch10", "ch11", >>> + "ch12", "ch13", "ch14", "ch15", >>> + "ch16", "ch17", "ch18", "ch19", >>> + "ch20", "ch21", "ch22", "ch23", >>> + "ch24"; >>> + clocks = <&cpg CPG_MOD 812>; >>> + power-domains = <&sysc 32>; >>> + resets = <&cpg 812>; >>> + phy-mode = "rgmii-txid"; >> >>Why not just "rgmii"? TX delay is a board specific detail, no? >> > > I admit I took this one straight from r8a7796 dtsi. > Would you like to me resend and change this? Yes, unless Simon would fix it while merging... > Thanks >j > >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + status = "disabled"; >>> }; >>> >>> csi20: csi2@fea8 { MBR, Sergei
Re: [PATCH v9 11/11] media: i2c: ov7670: Fully set mbus frame fmt
Hello again, On Tue, Feb 20, 2018 at 09:58:57AM +0100, jacopo mondi wrote: > Hi Laurent, > > On Mon, Feb 19, 2018 at 09:19:32PM +0200, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patch. > > > > On Monday, 19 February 2018 18:59:44 EET Jacopo Mondi wrote: > > > The sensor driver sets mbus format colorspace information and sizes, > > > but not ycbcr encoding, quantization and xfer function. When supplied > > > with an badly initialized mbus frame format structure, those fields > > > need to be set explicitly not to leave them uninitialized. This is > > > tested by v4l2-compliance, which supplies a mbus format description > > > structure and checks for all fields to be properly set. > > > > > > Without this commit, v4l2-compliance fails when testing formats with: > > > fail: v4l2-test-formats.cpp(335): ycbcr_enc >= 0xff > > > > > > Signed-off-by: Jacopo Mondi > > > --- > > > drivers/media/i2c/ov7670.c | 4 > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c > > > index 25b26d4..61c472e 100644 > > > --- a/drivers/media/i2c/ov7670.c > > > +++ b/drivers/media/i2c/ov7670.c > > > @@ -996,6 +996,10 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev > > > *sd, fmt->height = wsize->height; > > > fmt->colorspace = ov7670_formats[index].colorspace; > > > > On a side note, if I'm not mistaken the colorspace field is set to SRGB for > > all entries. Shouldn't you hardcode it here and remove the field ? > > > > > + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > > > + fmt->quantization = V4L2_QUANTIZATION_DEFAULT; > > > + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT; > > > > How about setting the values explicitly instead of relying on defaults ? > > That > > would be V4L2_YCBCR_ENC_601, V4L2_QUANTIZATION_LIM_RANGE and > > V4L2_XFER_FUNC_SRGB. And could you then check a captured frame to see if the > > sensor outputs limited or full range ? > > > > This actually makes me wonder if those informations (ycbcb_enc, > quantization and xfer_func) shouldn't actually be part of the > supported format list... I blindly added those default fields in the > try_fmt function, but I doubt they applies to all supported formats. > > Eg. the sensor supports YUYV as well as 2 RGB encodings (RGB444 and > RGB 565) and 1 raw format (BGGR). I now have a question here: > > 1) ycbcr_enc transforms non-linear R'G'B' to Y'CbCr: does this > applies to RGB and raw formats? I don't think so, and what value is > the correct one for the ycbcr_enc field in this case? I assume > xfer_func and quantization applies to all formats instead.. > What if I repropose this as separate patch not part of the CEU series in order not to hold back v10 (which I hope will be the last CEU iteration)? > Thanks >j > > > > info->format = *fmt; > > > > > > return 0; > > > > -- > > Regards, > > > > Laurent Pinchart > >
Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
Hi Rob, On Wed, Feb 21, 2018 at 4:23 PM, Rob Herring wrote: > On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven > wrote: >> You missed one fix I have in my topic/overlays branch >> https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=150f95b9dec77ce371c229f7ac4d6dd8620bef4a > > Are you planning to try to upstream all this? If not, I'll get Frank Not really. But I do need a way to load DT overlays at runtime, for testing hardware on expansion connectors. > to keep changing the overlay API to make carrying it out of tree more > painful. :) He already did a very good job w.r.t. that in v4.15-rc1 ;^) 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
[PATCH] watchdog: renesas_wdt: Blacklist early R-Car Gen2 SoCs
On early revisions of some R-Car Gen2 SoCs, and depending on SMP configuration, the system may fail to restart on watchdog time-out, and lock up instead. Specifically: - On R-Car H2 ES1.0 and M2-W ES1.0, watchdog restart fails unless only the first CPU core is in use (using e.g. the "maxcpus=1" kernel commandline option). - On R-Car V2H ES1.1, watchdog restart fails unless SMP is disabled completely (using CONFIG_SMP=n during build configuration, or using the "nosmp" or "maxcpus=0" kernel commandline options). Prevent using the watchdog in impacted cases by blacklisting the affected SoCs, using the minimum known working revisions (ES2.0 on R-Car H2, and ES3.0 on M2-W), and taking the actual SMP software configuration into account. Signed-off-by: Geert Uytterhoeven --- To be folded into Fabrizio Castro's "watchdog: renesas_wdt: Add R-Car Gen2 support". Note that I cannot use IS_ENABLED(CONFIG_SMP), as setup_max_cpus does not exist for CONFIG_SMP=n. Any reports on R-Car M2-W ES2.0 and V2H ES2.0+ are welcomed! As the failure is due to an integration issue, and the watchdog itself is working fine, an alternative solution would be to move the check to the code that installs the reset trigger ("soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2"). However, doing so would mean that: 1. The user could enable and seemingly use the watchdog, but watchdog timeout would not restart the system, 2. The same check should be done before installing the new reset vector ("ARM: shmobile: rcar-gen2: Add watchdog support"), too, else onlining CPU0 would fail once the watchdog has timed out. --- drivers/watchdog/renesas_wdt.c | 43 ++ 1 file changed, 43 insertions(+) diff --git a/drivers/watchdog/renesas_wdt.c b/drivers/watchdog/renesas_wdt.c index 0f88614797c36022..4c8e8d2600a922a5 100644 --- a/drivers/watchdog/renesas_wdt.c +++ b/drivers/watchdog/renesas_wdt.c @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #define RWTCNT 0 @@ -131,6 +133,44 @@ static const struct watchdog_ops rwdt_ops = { .restart = rwdt_restart, }; +#if defined(CONFIG_ARCH_RCAR_GEN2) && defined(CONFIG_SMP) +/* + * Watchdog-reset integration is broken on early revisions of R-Car Gen2 SoCs + */ +static const struct soc_device_attribute rwdt_quirks_match[] = { + { + .soc_id = "r8a7790", + .revision = "ES1.*", + .data = (void *)1, /* needs single CPU */ + }, { + .soc_id = "r8a7791", + .revision = "ES[12].*", + .data = (void *)1, /* needs single CPU */ + }, { + .soc_id = "r8a7792", + .revision = "*", + .data = (void *)0, /* needs SMP disabled */ + }, + { /* sentinel */ } +}; + +static bool rwdt_blacklisted(struct device *dev) +{ + const struct soc_device_attribute *attr; + + attr = soc_device_match(rwdt_quirks_match); + if (attr && setup_max_cpus > (uintptr_t)attr->data) { + dev_info(dev, "Watchdog blacklisted on %s %s\n", attr->soc_id, +attr->revision); + return true; + } + + return false; +} +#else /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */ +static inline bool rwdt_blacklisted(struct device *dev) { return false; } +#endif /* !CONFIG_ARCH_RCAR_GEN2 || !CONFIG_SMP */ + static int rwdt_probe(struct platform_device *pdev) { struct rwdt_priv *priv; @@ -139,6 +179,9 @@ static int rwdt_probe(struct platform_device *pdev) unsigned long clks_per_sec; int ret, i; + if (rwdt_blacklisted(&pdev->dev)) + return -ENODEV; + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM; -- 2.7.4
Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
On Wed, Feb 21, 2018 at 4:21 AM, Geert Uytterhoeven wrote: > Hi Laurent, > > On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart > wrote: >> From: Pantelis Antoniou >> >> Changesets are very powerful, but the lack of a helper API >> makes using them cumbersome. Introduce a simple copy based >> API that makes things considerably easier. >> >> To wit, adding a property using the raw API. >> >> struct property *prop; >> prop = kzalloc(sizeof(*prop)), GFP_KERNEL); >> prop->name = kstrdup("compatible"); >> prop->value = kstrdup("foo,bar"); >> prop->length = strlen(prop->value) + 1; >> of_changeset_add_property(ocs, np, prop); >> >> while using the helper API >> >> of_changeset_add_property_string(ocs, np, "compatible", >> "foo,bar"); >> >> Signed-off-by: Pantelis Antoniou >> [Fixed memory leak in __of_changeset_add_update_property_copy()] >> Signed-off-by: Laurent Pinchart > > You missed one fix I have in my topic/overlays branch > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=150f95b9dec77ce371c229f7ac4d6dd8620bef4a Are you planning to try to upstream all this? If not, I'll get Frank to keep changing the overlay API to make carrying it out of tree more painful. :) Rob
Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling
On 02/21/18 16:16, jacopo mondi wrote: >>> static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = { >>> - .enum_mbus_code = ov772x_enum_mbus_code, >>> - .get_selection = ov772x_get_selection, >>> - .get_fmt= ov772x_get_fmt, >>> - .set_fmt= ov772x_set_fmt, >>> + .enum_frame_interval= ov772x_enum_frame_interval, >>> + .enum_mbus_code = ov772x_enum_mbus_code, >>> + .get_selection = ov772x_get_selection, >>> + .get_fmt= ov772x_get_fmt, >>> + .set_fmt= ov772x_set_fmt, >> >> Shouldn't these last four ops be added in the previous patch? >> They don't have anything to do with the frame interval support. >> > > If you look closely you'll notice I have just re-aligned them, since I > was at there to add enum_frame_interval operation Ah, sorry. I missed that. Never mind then :-) Hans > >> Anyway, after taking care of the memsets and these four ops you can add >> my: >> >> Acked-by: Hans Verkuil > > Thanks >j >
Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling
Hi Hans, On Wed, Feb 21, 2018 at 01:12:14PM +0100, Hans Verkuil wrote: [snip] > > +static int ov772x_g_frame_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_frame_interval *ival) > > +{ > > + struct ov772x_priv *priv = to_ov772x(sd); > > + struct v4l2_fract *tpf = &ival->interval; > > + > > + memset(ival->reserved, 0, sizeof(ival->reserved)); > > This memset... > > > + tpf->numerator = 1; > > + tpf->denominator = priv->fps; > > + > > + return 0; > > +} > > + > > +static int ov772x_s_frame_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_frame_interval *ival) > > +{ > > + struct ov772x_priv *priv = to_ov772x(sd); > > + struct v4l2_fract *tpf = &ival->interval; > > + > > + memset(ival->reserved, 0, sizeof(ival->reserved)); > > ... and this memset can be dropped. The core code will memset this for you. > > I see! Ok, I'll drop them in v10 > > + > > + return ov772x_set_frame_rate(priv, tpf, priv->cfmt, priv->win); > > +} > > + > > static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl) > > { > > struct ov772x_priv *priv = container_of(ctrl->handler, > > @@ -757,6 +905,7 @@ static int ov772x_set_params(struct ov772x_priv *priv, > > const struct ov772x_win_size *win) > > { > > struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > > + struct v4l2_fract tpf; > > int ret; > > u8 val; > > > > @@ -885,6 +1034,13 @@ static int ov772x_set_params(struct ov772x_priv *priv, > > if (ret < 0) > > goto ov772x_set_fmt_error; > > > > + /* COM4, CLKRC: Set pixel clock and framerate. */ > > + tpf.numerator = 1; > > + tpf.denominator = priv->fps; > > + ret = ov772x_set_frame_rate(priv, &tpf, cfmt, win); > > + if (ret < 0) > > + goto ov772x_set_fmt_error; > > + > > /* > > * set COM8 > > */ > > @@ -1043,6 +1199,24 @@ static const struct v4l2_subdev_core_ops > > ov772x_subdev_core_ops = { > > .s_power= ov772x_s_power, > > }; > > > > +static int ov772x_enum_frame_interval(struct v4l2_subdev *sd, > > + struct v4l2_subdev_pad_config *cfg, > > + struct v4l2_subdev_frame_interval_enum > > *fie) > > +{ > > + if (fie->pad || fie->index >= OV772X_N_FRAME_INTERVALS) > > + return -EINVAL; > > + > > + if (fie->width != VGA_WIDTH && fie->width != QVGA_WIDTH) > > + return -EINVAL; > > + if (fie->height != VGA_HEIGHT && fie->height != QVGA_HEIGHT) > > + return -EINVAL; > > + > > + fie->interval.numerator = 1; > > + fie->interval.denominator = ov772x_frame_intervals[fie->index]; > > + > > + return 0; > > +} > > + > > static int ov772x_enum_mbus_code(struct v4l2_subdev *sd, > > struct v4l2_subdev_pad_config *cfg, > > struct v4l2_subdev_mbus_code_enum *code) > > @@ -1055,14 +1229,17 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev > > *sd, > > } > > > > static const struct v4l2_subdev_video_ops ov772x_subdev_video_ops = { > > - .s_stream = ov772x_s_stream, > > + .s_stream = ov772x_s_stream, > > + .s_frame_interval = ov772x_s_frame_interval, > > + .g_frame_interval = ov772x_g_frame_interval, > > }; > > > > static const struct v4l2_subdev_pad_ops ov772x_subdev_pad_ops = { > > - .enum_mbus_code = ov772x_enum_mbus_code, > > - .get_selection = ov772x_get_selection, > > - .get_fmt= ov772x_get_fmt, > > - .set_fmt= ov772x_set_fmt, > > + .enum_frame_interval= ov772x_enum_frame_interval, > > + .enum_mbus_code = ov772x_enum_mbus_code, > > + .get_selection = ov772x_get_selection, > > + .get_fmt= ov772x_get_fmt, > > + .set_fmt= ov772x_set_fmt, > > Shouldn't these last four ops be added in the previous patch? > They don't have anything to do with the frame interval support. > If you look closely you'll notice I have just re-aligned them, since I was at there to add enum_frame_interval operation > Anyway, after taking care of the memsets and these four ops you can add > my: > > Acked-by: Hans Verkuil Thanks j
Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver
On 02/21/18 13:29, Laurent Pinchart wrote: > Hi Hans, > > On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote: >> On 02/19/18 17:59, Jacopo Mondi wrote: >>> Add driver for Renesas Capture Engine Unit (CEU). >>> >>> The CEU interface supports capturing 'data' (YUV422) and 'images' >>> (NV[12|21|16|61]). >>> >>> This driver aims to replace the soc_camera-based sh_mobile_ceu one. >>> >>> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ >>> platform GR-Peach. >>> >>> Tested with ov7725 camera sensor on SH4 platform Migo-R. >>> >>> Signed-off-by: Jacopo Mondi >>> Reviewed-by: Laurent Pinchart >>> --- >>> >>> drivers/media/platform/Kconfig |9 + >>> drivers/media/platform/Makefile |1 + >>> drivers/media/platform/renesas-ceu.c | 1661 + >>> 3 files changed, 1671 insertions(+) >>> create mode 100644 drivers/media/platform/renesas-ceu.c >> >> >> >>> +static int ceu_s_input(struct file *file, void *priv, unsigned int i) >>> +{ >>> + struct ceu_device *ceudev = video_drvdata(file); >>> + struct ceu_subdev *ceu_sd_old; >>> + int ret; >>> + >>> + if (i >= ceudev->num_sd) >>> + return -EINVAL; >>> + >>> + if (vb2_is_streaming(&ceudev->vb2_vq)) >>> + return -EBUSY; >>> + >>> + if (i == ceudev->sd_index) >>> + return 0; >>> + >>> + ceu_sd_old = ceudev->sd; >>> + ceudev->sd = &ceudev->subdevs[i]; >>> + >>> + /* Make sure we can generate output image formats. */ >>> + ret = ceu_init_formats(ceudev); >> >> Why is this done for every s_input? I would expect that this is done only >> once for each subdev. >> >> I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix >> is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev. >> I think I prefer that over configuring a new default format every time you >> switch inputs. > > What does userspace expect today ? I don't think we document anywhere that > formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is > very vague: > > "To select a video input applications store the number of the desired input > in > an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer. > Side effects are possible. For example inputs may support different video > standards, so the driver may implicitly switch the current standard. Because > of these possible side effects applications must select an input before > querying or negotiating any other parameters." > > I understand that as meaning "anything can happen when you switch inputs, so > be prepared to reconfigure everything explicitly without assuming any > particular parameter to have a sane value". > >> This code will work for two subdevs with exactly the same >> formats/properties. But switching between e.g. a sensor and a video >> receiver will leave things in an inconsistent state as far as I can see. >> >> E.g. if input 1 is the video receiver then switching to that input and >> running 'v4l2-ctl -V' will show the sensor format, not the video receiver >> format. > > I agree that the format should be consistent with the selected input, as > calling VIDIOC_S_INPUT immediately followed by a stream start sequence > without > configuring formats should work (but produce a format that is not known to > userspace). My question is whether we should reset to a default format for > the > newly select input, or switch back to the previously set format. I'd tend to > go for the former to keep as little state as possible, but I'm open to > counter-proposals. What to do is up to the driver. My personal preference is to make it persistent per input, but that's just me. I won't block the other approach (resetting it to a default). Be aware that for video receivers the format depends on the current selected standard as well. I think the code does that correctly already, but it would be good to verify if possible. BTW, I think it is right that the spec isn't specific about what changes when you switch inputs. It can be quite complex for drivers to handle this and it is not unreasonable in my view for userspace to just assume it needs to configure from scratch after switching inputs. Regards, Hans > >>> + if (ret) { >>> + ceudev->sd = ceu_sd_old; >>> + return -EINVAL; >>> + } >>> + >>> + /* now that we're sure we can use the sensor, power off the old one. */ >>> + v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0); >>> + v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1); >>> + >>> + ceudev->sd_index = i; >>> + >>> + return 0; >>> +} >> >> The remainder of this driver looks good. >
Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver
Hi Hans, On Wednesday, 21 February 2018 14:03:24 EET Hans Verkuil wrote: > On 02/19/18 17:59, Jacopo Mondi wrote: > > Add driver for Renesas Capture Engine Unit (CEU). > > > > The CEU interface supports capturing 'data' (YUV422) and 'images' > > (NV[12|21|16|61]). > > > > This driver aims to replace the soc_camera-based sh_mobile_ceu one. > > > > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ > > platform GR-Peach. > > > > Tested with ov7725 camera sensor on SH4 platform Migo-R. > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Laurent Pinchart > > --- > > > > drivers/media/platform/Kconfig |9 + > > drivers/media/platform/Makefile |1 + > > drivers/media/platform/renesas-ceu.c | 1661 + > > 3 files changed, 1671 insertions(+) > > create mode 100644 drivers/media/platform/renesas-ceu.c > > > > > +static int ceu_s_input(struct file *file, void *priv, unsigned int i) > > +{ > > + struct ceu_device *ceudev = video_drvdata(file); > > + struct ceu_subdev *ceu_sd_old; > > + int ret; > > + > > + if (i >= ceudev->num_sd) > > + return -EINVAL; > > + > > + if (vb2_is_streaming(&ceudev->vb2_vq)) > > + return -EBUSY; > > + > > + if (i == ceudev->sd_index) > > + return 0; > > + > > + ceu_sd_old = ceudev->sd; > > + ceudev->sd = &ceudev->subdevs[i]; > > + > > + /* Make sure we can generate output image formats. */ > > + ret = ceu_init_formats(ceudev); > > Why is this done for every s_input? I would expect that this is done only > once for each subdev. > > I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix > is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev. > I think I prefer that over configuring a new default format every time you > switch inputs. What does userspace expect today ? I don't think we document anywhere that formats are stored per-input in drivers. The VIDIOC_S_INPUT documentation is very vague: "To select a video input applications store the number of the desired input in an integer and call the VIDIOC_S_INPUT ioctl with a pointer to this integer. Side effects are possible. For example inputs may support different video standards, so the driver may implicitly switch the current standard. Because of these possible side effects applications must select an input before querying or negotiating any other parameters." I understand that as meaning "anything can happen when you switch inputs, so be prepared to reconfigure everything explicitly without assuming any particular parameter to have a sane value". > This code will work for two subdevs with exactly the same > formats/properties. But switching between e.g. a sensor and a video > receiver will leave things in an inconsistent state as far as I can see. > > E.g. if input 1 is the video receiver then switching to that input and > running 'v4l2-ctl -V' will show the sensor format, not the video receiver > format. I agree that the format should be consistent with the selected input, as calling VIDIOC_S_INPUT immediately followed by a stream start sequence without configuring formats should work (but produce a format that is not known to userspace). My question is whether we should reset to a default format for the newly select input, or switch back to the previously set format. I'd tend to go for the former to keep as little state as possible, but I'm open to counter-proposals. > > + if (ret) { > > + ceudev->sd = ceu_sd_old; > > + return -EINVAL; > > + } > > + > > + /* now that we're sure we can use the sensor, power off the old one. */ > > + v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0); > > + v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1); > > + > > + ceudev->sd_index = i; > > + > > + return 0; > > +} > > The remainder of this driver looks good. -- Regards, Laurent Pinchart
Re: [PATCH v4 04/16] of: changesets: Introduce changeset helper methods
Hi Geert, On Wednesday, 21 February 2018 12:21:50 EET Geert Uytterhoeven wrote: > On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote: > > From: Pantelis Antoniou > > > > Changesets are very powerful, but the lack of a helper API > > makes using them cumbersome. Introduce a simple copy based > > API that makes things considerably easier. > > > > To wit, adding a property using the raw API. > > > > struct property *prop; > > prop = kzalloc(sizeof(*prop)), GFP_KERNEL); > > prop->name = kstrdup("compatible"); > > prop->value = kstrdup("foo,bar"); > > prop->length = strlen(prop->value) + 1; > > of_changeset_add_property(ocs, np, prop); > > > > while using the helper API > > > > of_changeset_add_property_string(ocs, np, "compatible", > > > > "foo,bar"); > > > > Signed-off-by: Pantelis Antoniou > > [Fixed memory leak in __of_changeset_add_update_property_copy()] > > Signed-off-by: Laurent Pinchart > > > > You missed one fix I have in my topic/overlays branch > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/co > mmit/?h=topic/overlays&id=150f95b9dec77ce371c229f7ac4d6dd8620bef4a > > --- a/include/linux/of.h > > +++ b/include/linux/of.h > > > > +/** > > + * of_changeset_add_property_u32 - Create a new u32 property > > + * > > + * @ocs: changeset pointer > > + * @np:device node pointer > > + * @name: name of the property > > + * @val: value in host endian format > > + * > > + * Adds a u32 property to the changeset. > > + * > > + * Returns zero on success, a negative error value otherwise. > > + */ > > +static inline int of_changeset_add_property_u32(struct of_changeset *ocs, > > + struct device_node *np, const char *name, u32 val) > > +{ > > + val = cpu_to_be32(val); > > You must use an intermediate, to avoid complaints from sparse: > > __be32 x = cpu_to_be32(val); > > > + return __of_changeset_add_update_property_copy(ocs, np, name, > > &val, > > + sizeof(val), false); > > s/val/x/ I'll fix both in the next version, thanks. > > +} > > + > > +/** > > + * of_changeset_update_property_u32 - Update u32 property > > + * > > + * @ocs: changeset pointer > > + * @np:device node pointer > > + * @name: name of the property > > + * @val: value in host endian format > > + * > > + * Updates a u32 property to the changeset. > > + * > > + * Returns zero on success, a negative error value otherwise. > > + */ > > +static inline int of_changeset_update_property_u32( > > + struct of_changeset *ocs, struct device_node *np, > > + const char *name, u32 val) > > +{ > > + val = cpu_to_be32(val); > > Oh, a new one. > > > + return __of_changeset_add_update_property_copy(ocs, np, name, > > &val, > > + sizeof(val), true); > > +} -- Regards, Laurent Pinchart
Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
Hi Geert, On Wednesday, 21 February 2018 12:26:45 EET Geert Uytterhoeven wrote: > On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote: > > From: Pantelis Antoniou > > > > Add an __of_node_dupv() private method and make __of_node_dup() use it. > > This is required for the subsequent changeset accessors which will > > make use of it. > > > > Signed-off-by: Pantelis Antoniou > > Signed-off-by: Laurent Pinchart > > > > --- > > > > drivers/of/dynamic.c | 29 +++-- > > 1 file changed, 23 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c > > index 7bb33d22b4e2..4ffd04925fdf 100644 > > --- a/drivers/of/dynamic.c > > +++ b/drivers/of/dynamic.c > > @@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property > > *prop, gfp_t allocflags)> > > } > > > > /** > > > > - * __of_node_dup() - Duplicate or create an empty device node > > dynamically. > > - * @fmt: Format string (plus vargs) for new full name of the device node > > + * __of_node_dupv() - Duplicate or create an empty device node > > dynamically. + * @fmt: Format string for new full name of the device node > > + * @vargs: va_list containing the arugments for the node full name > > > > * > > * Create an device tree node, either by duplicating an empty node or by > > allocating * an empty one suitable for further modification. The node > > data are> > > @@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property > > *prop, gfp_t allocflags)> > > * OF_DETACHED bits set. Returns the newly allocated node or NULL on out > > of > > * memory error. > > */ > > > > -struct device_node *__of_node_dup(const struct device_node *np, const > > char *fmt, ...) +struct device_node *__of_node_dupv(const struct > > device_node *np, > > static, cfr. > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/co > mmit/?h=topic/overlays&id=c45324e1807dd708344c9a478b777b68aca11cdf I'll fix that in the next version. -- Regards, Laurent Pinchart
Re: [PATCH v9 07/11] media: i2c: ov772x: Support frame interval handling
On 02/19/18 17:59, Jacopo Mondi wrote: > Add support to ov772x driver for frame intervals handling and enumeration. > Tested with 10MHz and 24MHz input clock at VGA and QVGA resolutions for > 10, 15 and 30 frame per second rates. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Laurent Pinchart > --- > drivers/media/i2c/ov772x.c | 212 > + > 1 file changed, 195 insertions(+), 17 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 23106d7..eba71d9 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -250,6 +250,7 @@ > #define AEC_1p2 0x10 /* 01: 1/2 window */ > #define AEC_1p4 0x20 /* 10: 1/4 window */ > #define AEC_2p3 0x30 /* 11: Low 2/3 window */ > +#define COM4_RESERVED 0x01 /* Reserved bit */ > > /* COM5 */ > #define AFR_ON_OFF 0x80 /* Auto frame rate control ON/OFF selection */ > @@ -267,6 +268,10 @@ > /* AEC max step control */ > #define AEC_NO_LIMIT0x01 /* 0 : AEC incease step has limit */ > /* 1 : No limit to AEC increase step */ > +/* CLKRC */ > + /* Input clock divider register */ > +#define CLKRC_RESERVED 0x80 /* Reserved bit */ > +#define CLKRC_DIV(n)((n) - 1) > > /* COM7 */ > /* SCCB Register Reset */ > @@ -373,6 +378,19 @@ > #define VERSION(pid, ver) ((pid<<8)|(ver&0xFF)) > > /* > + * PLL multipliers > + */ > +struct { > + unsigned int mult; > + u8 com4; > +} ov772x_pll[] = { > + { 1, PLL_BYPASS, }, > + { 4, PLL_4x, }, > + { 6, PLL_6x, }, > + { 8, PLL_8x, }, > +}; > + > +/* > * struct > */ > > @@ -388,6 +406,7 @@ struct ov772x_color_format { > struct ov772x_win_size { > char *name; > unsigned char com7_bit; > + unsigned int sizeimage; > struct v4l2_rect rect; > }; > > @@ -404,6 +423,7 @@ struct ov772x_priv { > unsigned shortflag_hflip:1; > /* band_filter = COM8[5] ? 256 - BDBASE : 0 */ > unsigned shortband_filter; > + unsigned int fps; > }; > > /* > @@ -487,27 +507,35 @@ static const struct ov772x_color_format ov772x_cfmts[] > = { > > static const struct ov772x_win_size ov772x_win_sizes[] = { > { > - .name = "VGA", > - .com7_bit = SLCT_VGA, > + .name = "VGA", > + .com7_bit = SLCT_VGA, > + .sizeimage = 510 * 748, > .rect = { > - .left = 140, > - .top = 14, > - .width = VGA_WIDTH, > - .height = VGA_HEIGHT, > + .left = 140, > + .top= 14, > + .width = VGA_WIDTH, > + .height = VGA_HEIGHT, > }, > }, { > - .name = "QVGA", > - .com7_bit = SLCT_QVGA, > + .name = "QVGA", > + .com7_bit = SLCT_QVGA, > + .sizeimage = 278 * 576, > .rect = { > - .left = 252, > - .top = 6, > - .width = QVGA_WIDTH, > - .height = QVGA_HEIGHT, > + .left = 252, > + .top= 6, > + .width = QVGA_WIDTH, > + .height = QVGA_HEIGHT, > }, > }, > }; > > /* > + * frame rate settings lists > + */ > +unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 }; > +#define OV772X_N_FRAME_INTERVALS ARRAY_SIZE(ov772x_frame_intervals) > + > +/* > * general function > */ > > @@ -574,6 +602,126 @@ static int ov772x_s_stream(struct v4l2_subdev *sd, int > enable) > return 0; > } > > +static int ov772x_set_frame_rate(struct ov772x_priv *priv, > + struct v4l2_fract *tpf, > + const struct ov772x_color_format *cfmt, > + const struct ov772x_win_size *win) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev); > + unsigned long fin = clk_get_rate(priv->clk); > + unsigned int fps = tpf->numerator ? > +tpf->denominator / tpf->numerator : > +tpf->denominator; > + unsigned int best_diff; > + unsigned int fsize; > + unsigned int pclk; > + unsigned int diff; > + unsigned int idx; > + unsigned int i; > + u8 clkrc = 0; > + u8 com4 = 0; > + int ret; > + > + /* Approximate to the closest supported frame interval. */ > + best_diff = ~0L; > + for (i = 0, idx = 0; i < OV772X_N_FRAME_INTERVALS; i++) { > + diff = abs(fps - ov772x_frame_intervals[i]); > + if (diff
Re: [PATCH v9 06/11] media: i2c: ov772x: Remove soc_camera dependencies
On 02/19/18 17:59, Jacopo Mondi wrote: > Remove soc_camera framework dependencies from ov772x sensor driver. > - Handle clock and gpios > - Register async subdevice > - Remove soc_camera specific g/s_mbus_config operations > - Change image format colorspace from JPEG to SRGB as the two use the > same colorspace information but JPEG makes assumptions on color > components quantization that do not apply to the sensor > - Remove sizes crop from get_selection as driver can't scale > - Add kernel doc to driver interface header file > - Adjust build system > > This commit does not remove the original soc_camera based driver as long > as other platforms depends on soc_camera-based CEU driver. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Laurent Pinchart > --- > drivers/media/i2c/Kconfig | 11 +++ > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/ov772x.c | 172 > ++--- > include/media/i2c/ov772x.h | 6 +- > 4 files changed, 133 insertions(+), 57 deletions(-) > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 9f18cd2..e71e968 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -645,6 +645,17 @@ config VIDEO_OV5670 > To compile this driver as a module, choose M here: the > module will be called ov5670. > > +config VIDEO_OV772X > + tristate "OmniVision OV772x sensor support" > + depends on I2C && VIDEO_V4L2 > + depends on MEDIA_CAMERA_SUPPORT > + ---help--- > + This is a Video4Linux2 sensor-level driver for the OmniVision > + OV772x camera. > + > + To compile this driver as a module, choose M here: the > + module will be called ov772x. > + > config VIDEO_OV7640 > tristate "OmniVision OV7640 sensor support" > depends on I2C && VIDEO_V4L2 > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index c0f94cd..b0489a1 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o > obj-$(CONFIG_VIDEO_OV6650) += ov6650.o > obj-$(CONFIG_VIDEO_OV7640) += ov7640.o > obj-$(CONFIG_VIDEO_OV7670) += ov7670.o > +obj-$(CONFIG_VIDEO_OV772X) += ov772x.o > obj-$(CONFIG_VIDEO_OV7740) += ov7740.o > obj-$(CONFIG_VIDEO_OV9650) += ov9650.o > obj-$(CONFIG_VIDEO_OV13858) += ov13858.o > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index 8063835..23106d7 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -1,6 +1,9 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * ov772x Camera Driver > * > + * Copyright (C) 2017 Jacopo Mondi > + * > * Copyright (C) 2008 Renesas Solutions Corp. > * Kuninori Morimoto > * > @@ -9,27 +12,25 @@ > * Copyright 2006-7 Jonathan Corbet > * Copyright (C) 2008 Magnus Damm > * Copyright (C) 2008, Guennadi Liakhovetski > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License version 2 as > - * published by the Free Software Foundation. > */ > > +#include > +#include > +#include > +#include > #include > #include > #include > -#include > #include > -#include > #include > #include > > #include > -#include > -#include > + > #include > -#include > +#include > #include > +#include > > /* > * register offset > @@ -393,8 +394,10 @@ struct ov772x_win_size { > struct ov772x_priv { > struct v4l2_subdevsubdev; > struct v4l2_ctrl_handler hdl; > - struct v4l2_clk *clk; > + struct clk *clk; > struct ov772x_camera_info*info; > + struct gpio_desc *pwdn_gpio; > + struct gpio_desc *rstb_gpio; > const struct ov772x_color_format *cfmt; > const struct ov772x_win_size *win; > unsigned shortflag_vflip:1; > @@ -409,7 +412,7 @@ struct ov772x_priv { > static const struct ov772x_color_format ov772x_cfmts[] = { > { > .code = MEDIA_BUS_FMT_YUYV8_2X8, > - .colorspace = V4L2_COLORSPACE_JPEG, > + .colorspace = V4L2_COLORSPACE_SRGB, > .dsp3 = 0x0, > .dsp4 = DSP_OFMT_YUV, > .com3 = SWAP_YUV, > @@ -417,7 +420,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { > }, > { > .code = MEDIA_BUS_FMT_YVYU8_2X8, > - .colorspace = V4L2_COLORSPACE_JPEG, > + .colorspace = V4L2_COLORSPACE_SRGB, > .dsp3 = UV_ON, > .dsp4 = DSP_OFMT_YUV, > .com3 = SWAP_YUV, > @@ -425,7 +428,7 @@ static const struct ov772x_color_format ov772x_cfmts[] = { > }, > { > .code = MEDIA_BUS_FMT_UYVY8_2X8, > - .c
Re: [PATCH v9 03/11] media: platform: Add Renesas CEU driver
On 02/19/18 17:59, Jacopo Mondi wrote: > Add driver for Renesas Capture Engine Unit (CEU). > > The CEU interface supports capturing 'data' (YUV422) and 'images' > (NV[12|21|16|61]). > > This driver aims to replace the soc_camera-based sh_mobile_ceu one. > > Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ > platform GR-Peach. > > Tested with ov7725 camera sensor on SH4 platform Migo-R. > > Signed-off-by: Jacopo Mondi > Reviewed-by: Laurent Pinchart > --- > drivers/media/platform/Kconfig |9 + > drivers/media/platform/Makefile |1 + > drivers/media/platform/renesas-ceu.c | 1661 > ++ > 3 files changed, 1671 insertions(+) > create mode 100644 drivers/media/platform/renesas-ceu.c > > +static int ceu_s_input(struct file *file, void *priv, unsigned int i) > +{ > + struct ceu_device *ceudev = video_drvdata(file); > + struct ceu_subdev *ceu_sd_old; > + int ret; > + > + if (i >= ceudev->num_sd) > + return -EINVAL; > + > + if (vb2_is_streaming(&ceudev->vb2_vq)) > + return -EBUSY; > + > + if (i == ceudev->sd_index) > + return 0; > + > + ceu_sd_old = ceudev->sd; > + ceudev->sd = &ceudev->subdevs[i]; > + > + /* Make sure we can generate output image formats. */ > + ret = ceu_init_formats(ceudev); Why is this done for every s_input? I would expect that this is done only once for each subdev. I also expect to see a ceu_set_default_fmt() call here. Or that the v4l2_pix is kept in ceu_subdev (i.e. per subdev) instead of a single fmt in cuedev. I think I prefer that over configuring a new default format every time you switch inputs. This code will work for two subdevs with exactly the same formats/properties. But switching between e.g. a sensor and a video receiver will leave things in an inconsistent state as far as I can see. E.g. if input 1 is the video receiver then switching to that input and running 'v4l2-ctl -V' will show the sensor format, not the video receiver format. > + if (ret) { > + ceudev->sd = ceu_sd_old; > + return -EINVAL; > + } > + > + /* now that we're sure we can use the sensor, power off the old one. */ > + v4l2_subdev_call(ceu_sd_old->v4l2_sd, core, s_power, 0); > + v4l2_subdev_call(ceudev->sd->v4l2_sd, core, s_power, 1); > + > + ceudev->sd_index = i; > + > + return 0; > +} The remainder of this driver looks good. Regards, Hans
Re: [PATCH v4 03/16] of: dynamic: Add __of_node_dupv()
Hi Laurent, On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Add an __of_node_dupv() private method and make __of_node_dup() use it. > This is required for the subsequent changeset accessors which will > make use of it. > > Signed-off-by: Pantelis Antoniou > Signed-off-by: Laurent Pinchart > --- > drivers/of/dynamic.c | 29 +++-- > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c > index 7bb33d22b4e2..4ffd04925fdf 100644 > --- a/drivers/of/dynamic.c > +++ b/drivers/of/dynamic.c > @@ -382,8 +382,9 @@ struct property *__of_prop_dup(const struct property > *prop, gfp_t allocflags) > } > > /** > - * __of_node_dup() - Duplicate or create an empty device node dynamically. > - * @fmt: Format string (plus vargs) for new full name of the device node > + * __of_node_dupv() - Duplicate or create an empty device node dynamically. > + * @fmt: Format string for new full name of the device node > + * @vargs: va_list containing the arugments for the node full name > * > * Create an device tree node, either by duplicating an empty node or by > allocating > * an empty one suitable for further modification. The node data are > @@ -391,17 +392,15 @@ struct property *__of_prop_dup(const struct property > *prop, gfp_t allocflags) > * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of > * memory error. > */ > -struct device_node *__of_node_dup(const struct device_node *np, const char > *fmt, ...) > +struct device_node *__of_node_dupv(const struct device_node *np, static, cfr. https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=c45324e1807dd708344c9a478b777b68aca11cdf 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 v4 04/16] of: changesets: Introduce changeset helper methods
Hi Laurent, On Wed, Feb 21, 2018 at 12:10 AM, Laurent Pinchart wrote: > From: Pantelis Antoniou > > Changesets are very powerful, but the lack of a helper API > makes using them cumbersome. Introduce a simple copy based > API that makes things considerably easier. > > To wit, adding a property using the raw API. > > struct property *prop; > prop = kzalloc(sizeof(*prop)), GFP_KERNEL); > prop->name = kstrdup("compatible"); > prop->value = kstrdup("foo,bar"); > prop->length = strlen(prop->value) + 1; > of_changeset_add_property(ocs, np, prop); > > while using the helper API > > of_changeset_add_property_string(ocs, np, "compatible", > "foo,bar"); > > Signed-off-by: Pantelis Antoniou > [Fixed memory leak in __of_changeset_add_update_property_copy()] > Signed-off-by: Laurent Pinchart You missed one fix I have in my topic/overlays branch https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/commit/?h=topic/overlays&id=150f95b9dec77ce371c229f7ac4d6dd8620bef4a > --- a/include/linux/of.h > +++ b/include/linux/of.h > +/** > + * of_changeset_add_property_u32 - Create a new u32 property > + * > + * @ocs: changeset pointer > + * @np:device node pointer > + * @name: name of the property > + * @val: value in host endian format > + * > + * Adds a u32 property to the changeset. > + * > + * Returns zero on success, a negative error value otherwise. > + */ > +static inline int of_changeset_add_property_u32(struct of_changeset *ocs, > + struct device_node *np, const char *name, u32 val) > +{ > + val = cpu_to_be32(val); You must use an intermediate, to avoid complaints from sparse: __be32 x = cpu_to_be32(val); > + return __of_changeset_add_update_property_copy(ocs, np, name, &val, > + sizeof(val), false); s/val/x/ > +} > + > +/** > + * of_changeset_update_property_u32 - Update u32 property > + * > + * @ocs: changeset pointer > + * @np:device node pointer > + * @name: name of the property > + * @val: value in host endian format > + * > + * Updates a u32 property to the changeset. > + * > + * Returns zero on success, a negative error value otherwise. > + */ > +static inline int of_changeset_update_property_u32( > + struct of_changeset *ocs, struct device_node *np, > + const char *name, u32 val) > +{ > + val = cpu_to_be32(val); Oh, a new one. > + return __of_changeset_add_update_property_copy(ocs, np, name, &val, > + sizeof(val), true); > +} Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 19/19] ARM64: dts: r8a77965: Add EtherAVB device node
Hi Sergei, On Tue, Feb 20, 2018 at 06:30:56PM +0300, Sergei Shtylyov wrote: > On 02/20/2018 06:12 PM, Jacopo Mondi wrote: > > > Populate the ethernet@e680 device node to enable Ethernet interface > > for R-Car M3-N (r8a77965) SoC. > > > > Signed-off-by: Jacopo Mondi > > Reviewed-by: Geert Uytterhoeven > > > > --- > > v1 -> v2: > > - Replace ALWAYS_ON power area identifier with numeric constant > > --- > > arch/arm64/boot/dts/renesas/r8a77965.dtsi | 43 > > ++- > > 1 file changed, 42 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > > index 55f05f7..c249895 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi > > @@ -520,7 +520,48 @@ > > }; > > > > avb: ethernet@e680 { > > - /* placeholder */ > > + compatible = "renesas,etheravb-r8a77965", > > +"renesas,etheravb-rcar-gen3"; > > + reg = <0 0xe680 0 0x800>, <0 0xe6a0 0 0x1>; > > + interrupts = , > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +, > > +; > > + interrupt-names = "ch0", "ch1", "ch2", "ch3", > > + "ch4", "ch5", "ch6", "ch7", > > + "ch8", "ch9", "ch10", "ch11", > > + "ch12", "ch13", "ch14", "ch15", > > + "ch16", "ch17", "ch18", "ch19", > > + "ch20", "ch21", "ch22", "ch23", > > + "ch24"; > > + clocks = <&cpg CPG_MOD 812>; > > + power-domains = <&sysc 32>; > > + resets = <&cpg 812>; > > + phy-mode = "rgmii-txid"; > >Why not just "rgmii"? TX delay is a board specific detail, no? > I admit I took this one straight from r8a7796 dtsi. Would you like to me resend and change this? Thanks j > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "disabled"; > > }; > > > > csi20: csi2@fea8 { > > > > MBR, Sergei
Re: [PATCH v4 01/16] dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings
On 2/21/2018 2:10 AM, Laurent Pinchart wrote: The Renesas R-Car Gen2 and Gen3 SoCs have internal LVDS encoders. Add corresponding device tree bindings. Signed-off-by: Laurent Pinchart Reviewed-by: Rob Herring --- Changes since v1: - Move the SoC name before the IP name in compatible strings - Rename parallel input to parallel RGB input - Fixed "renesas,r8a7743-lvds" description - Document the resets property - Fixed typo --- .../bindings/display/bridge/renesas,lvds.txt | 56 ++ MAINTAINERS| 1 + 2 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt new file mode 100644 index ..2b19ce51ec07 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt @@ -0,0 +1,56 @@ +Renesas R-Car LVDS Encoder +== + +These DT bindings describe the LVDS encoder embedded in the Renesas R-Car +Gen2, R-Car Gen3 and RZ/G SoCs. + +Required properties: + +- compatible : Shall contain one of + - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders + - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders + - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders + - "renesas,r8a7793-lvds" for R8A7791 (R-Car M2-N) compatible LVDS encoders + - "renesas,r8a7795-lvds" for R8A7795 (R-Car H3) compatible LVDS encoders + - "renesas,r8a7796-lvds" for R8A7796 (R-Car M3-W) compatible LVDS encoders + +- reg: Base address and length for the memory-mapped registers +- clocks: A phandle + clock-specifier pair for the functional clock +- resets: A phandle + reset specifier for the module reset + +Required nodes: + +The LVDS encoder has two video ports. Their connections are modelled using the +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt. + +- Video port 0 corresponds to the parallel RGB input +- Video port 1 corresponds to the LVDS output + +Each port shall have a single endpoint. + + +Example: + + lvds0: lvds@feb9 { "lvds-encoder@feb9" maybe? And do we need a # in the label if the encoder is alone in DT anyway? [...] MBR, Sergei
Re: [PATCH v4 14/16] ARM: dts: r8a7794: Convert to new DU DT bindings
Hello! On 2/21/2018 2:10 AM, Laurent Pinchart wrote: The DU DT bindings have been updated to drop the reg-names property. Update the r8a7792 device tree accordingly. Apparently r8a7794. Signed-off-by: Laurent Pinchart [...] MBR, Sergei
Re: [PATCH v4 07/16] i2c: demux: Use changeset helpers for clarity
On Wed, Feb 21, 2018 at 01:10:37AM +0200, Laurent Pinchart wrote: > From: Pantelis Antoniou > > The changeset helpers are easier to use, use them instead of > using the static property. > > Signed-off-by: Pantelis Antoniou > Acked-by: Wolfram Sang My ack still holds. Good luck pushing this series! signature.asc Description: PGP signature