Hello On 30/06/2022 11:09, Tommaso Merciai wrote: > Hi Sakari, > > On Thu, Jun 30, 2022 at 12:50:05PM +0300, Sakari Ailus wrote: >> Hi Tommaso, >> >> On Thu, Jun 30, 2022 at 11:16:13AM +0200, Tommaso Merciai wrote: >>> Hi Sakari, >>> >>> On Thu, Jun 30, 2022 at 12:12:47PM +0300, Sakari Ailus wrote: >>>> On Thu, Jun 30, 2022 at 11:02:32AM +0200, Tommaso Merciai wrote: >>>>> On Thu, Jun 30, 2022 at 10:07:19AM +0200, Krzysztof Kozlowski wrote: >>>>>> On 30/06/2022 09:45, Tommaso Merciai wrote: >>>>>>> Add documentation of device tree in YAML schema for the OV5693 >>>>>>> CMOS image sensor from Omnivision >>>>>>> >>>>>>> Signed-off-by: Tommaso Merciai <tommaso.merciai@xxxxxxxxxxxxxxxxxxxx> >>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>>>>>> Reviewed-by: Sakari Ailus <sakari.ailus@xxxxxx> >>>>>> How Sakari's tag appeared here? There was no email from him. >>>>> Sakari made me some review on v2, but I think he forgot to add the mailing >>>>> list in cc. ( I suppose :) ) >>>>> >>>>> Let me know if I need to remove this. >>>> You're only supposed to put these tags into patches if you get them in >>>> written form as part of the review, signalling acceptance of the patch in >>>> various forms. Just commenting a patch does not imply this. >>>> >>>> Please also see Documentation/process/submitting-patches.rst for more >>>> information on how to use the tags. >>> Thanks for sharing this. My bad. >>> I remove your tags. >> The patches themselves seem fine. I'd just drop the 4th patch or at least >> come up with a better name for ov5693_hwcfg() --- you're acquiring >> resources there, and that generally fits well for probe. The code is fine >> already. > Then we don't need v5 with your reviewed tags removed? > > I think the patch4 is needed to add dts support properly. > Also this contains devm_clk_get_optional fix suggested by Jacopo and > support for ACPI-based platforms that specify the clock frequency by > using the "clock-frequency" property instead of specifying a clock > provider reference. I agree patch 4 in some form is needed - I didn't do the clock handling particularly well in this driver, and though it's ostensibly an ACPI driver it wouldn't actually work with a "normal" ACPI, but just with the cio2-bridge-repaired style. So the changes to the clock handling logic are welcome and needed I think. whether it needs to go into a separate function I don't particularly mind either way. > > Some suggestion on the function name? > > Thanks, > Tommaso > >> -- >> Sakari Ailus