Re: [PATCH v3 4/8] serdev: add of_ helper to get serdev controller

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 15/03/2024 19:49, Ayush Singh wrote:
> From: Vaishnav M A <vaishnav@xxxxxxxxxxxxxxx>
> 
> add of_find_serdev_controller_by_node to obtain a
> serdev_controller from the device_node, which
> can help if the serdev_device is not described
> over device tree and instantiation of the device
> happens from a different driver, for the same purpose
> an option to not delete an empty serdev controller
> is added.

Don't make it difficult for us to read your commit msgs.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Vaishnav M A <vaishnav@xxxxxxxxxxxxxxx>
> Signed-off-by: Ayush Singh <ayushdevel1325@xxxxxxxxx>
> ---
>  drivers/tty/serdev/core.c | 19 +++++++++++++++++++
>  include/linux/serdev.h    |  4 ++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 613cb356b918..5b5b3f2b2e42 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -555,6 +555,19 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_OF)
> +struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node)
> +{
> +	struct device *dev = bus_find_device_by_of_node(&serdev_bus_type, node);
> +
> +	if (!dev)
> +		return NULL;
> +
> +	return (dev->type == &serdev_ctrl_type) ? to_serdev_controller(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(of_find_serdev_controller_by_node);
> +#endif
> +
>  #ifdef CONFIG_ACPI
>  
>  #define SERDEV_ACPI_MAX_SCAN_DEPTH 32
> @@ -785,6 +798,12 @@ int serdev_controller_add(struct serdev_controller *ctrl)
>  
>  	pm_runtime_enable(&ctrl->dev);
>  
> +	/* provide option to not delete a serdev controller without devices
> +	 * if property is present
> +	 */
> +	if (device_property_present(&ctrl->dev, "force-empty-serdev-controller"))

How is this related to topic of adding helper? Why are you adding some
undocumented properties?

No, it's the same in other patches - you combine unrelated goals into
one patch. Please read carefully submitting patches document how to
organize your patchset.


Best regards,
Krzysztof