Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: CvfHlA+0aDqLdUK9ChXLaQ2yxJphb9dOycWDFLhmo8GEyLeoll6VNSxi8Y0w4DAJ0MBaJ9QReZ
 B9GVjt5rqJ4g==
IronPort-SDR: UigXKErjXKs6WPtNtfQFDGrTuG5eYxjya2rRfq29NkseFbmn3DtX2yLAT01AfBdZ9uCNyPJs5S
 zFeu9PVdnCSg==
Date:   Tue, 30 Jun 2020 17:22:54 +0300
From:   Heikki Krogerus <heikki.krogerus@linux.intel.com>
To:     Prashant Malani <pmalani@chromium.org>
Cc:     linux-kernel@vger.kernel.org, Benson Leung <bleung@chromium.org>,
        Enric Balletbo i Serra <enric.balletbo@collabora.com>,
        Guenter Roeck <groeck@chromium.org>
Subject: Re: [PATCH v4 1/2] platform/chrome: cros_ec_typec: Use workqueue for
 port update
Message-ID: <20200630142254.GD856968@kuha.fi.intel.com>
References: <20200629211329.2185342-1-pmalani@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200629211329.2185342-1-pmalani@chromium.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jun 29, 2020 at 02:13:27PM -0700, Prashant Malani wrote:
> Use a work queue to call the port update routines, instead of doing it
> directly in the PD notifier callback. This will prevent other drivers
> with PD notifier callbacks from being blocked on the port update routine
> completing.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> 
> Changes in v4:
> - Removed extra newline.
> - Moved both variable declarations into one line.
> 
> Changes in v3:
> - Use new 100 character line length limit.
> 
> Changes in v2:
> - No changes.
> 
>  drivers/platform/chrome/cros_ec_typec.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 0c041b79cbba..69c4118e280c 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -58,6 +58,7 @@ struct cros_typec_data {
>  	/* Array of ports, indexed by port number. */
>  	struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
>  	struct notifier_block nb;
> +	struct work_struct port_work;
>  };
>  
>  static int cros_typec_parse_port_props(struct typec_capability *cap,
> @@ -619,18 +620,24 @@ static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
>  	return 0;
>  }
>  
> -static int cros_ec_typec_event(struct notifier_block *nb,
> -			       unsigned long host_event, void *_notify)
> +static void cros_typec_port_work(struct work_struct *work)
>  {
> -	struct cros_typec_data *typec = container_of(nb, struct cros_typec_data,
> -						     nb);
> -	int ret, i;
> +	struct cros_typec_data *typec = container_of(work, struct cros_typec_data, port_work);
> +	int i, ret;
>  
>  	for (i = 0; i < typec->num_ports; i++) {
>  		ret = cros_typec_port_update(typec, i);
>  		if (ret < 0)
>  			dev_warn(typec->dev, "Update failed for port: %d\n", i);
>  	}
> +}
> +
> +static int cros_ec_typec_event(struct notifier_block *nb,
> +			       unsigned long host_event, void *_notify)
> +{
> +	struct cros_typec_data *typec = container_of(nb, struct cros_typec_data, nb);
> +
> +	schedule_work(&typec->port_work);
>  
>  	return NOTIFY_OK;
>  }
> @@ -689,6 +696,12 @@ static int cros_typec_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> +	INIT_WORK(&typec->port_work, cros_typec_port_work);
> +
> +	/*
> +	 * Safe to call port update here, since we haven't registered the
> +	 * PD notifier yet.
> +	 */
>  	for (i = 0; i < typec->num_ports; i++) {
>  		ret = cros_typec_port_update(typec, i);
>  		if (ret < 0)
> -- 
> 2.27.0.212.ge8ba1cc988-goog

thanks,

-- 
heikki