From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E2712C433E0 for ; Wed, 1 Jul 2020 10:51:01 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A9B9F20747 for ; Wed, 1 Jul 2020 10:51:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="aKQ6W/1I"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="ebE0Vf2T" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A9B9F20747 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=NKs25nG1Ca3WgYAf+uUmx8DOn1oXWQAElo9RUcwHKTk=; b=aKQ6W/1IXayzhStZA7bbLjNbm ITX6yQBnWE7cwfs7P9DArd6OW7qmzKUT7DLAULcT8XwC69BRs/1drvhaLd9fKaalWvuw7tE90zVR5 phBjUHCA+xb+axjjbpd9h0lNFTt5mfWkn4M1swSeHG1xU+lr5IjmfEwJf1/mYhdiqobGORjsiYpOC Q32+pebTqs59/h9FGapxTEGiVAZvvy5eiGGzQYJHgxQrx/T8Q4fXvCeNkmpG2kQcadc/Gfe9YNHee d9G1WPoppdg5S/Kh/jugbl5cSIxtX3LfSwEfiuCi6AgHwH+SnReS1LG+xa8Be8YFbVBItH8lPaXRy ggDMXj0Kg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqaJM-0000BT-T9; Wed, 01 Jul 2020 10:49:52 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqaJJ-0000Am-UJ for linux-arm-kernel@lists.infradead.org; Wed, 01 Jul 2020 10:49:50 +0000 Received: from localhost (p54b334c2.dip0.t-ipconnect.de [84.179.52.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4F12B20722; Wed, 1 Jul 2020 10:49:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593600589; bh=fBRyFwIDzERIG5z/DV53hFH9bjjSps7WeBweGXWWG4s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ebE0Vf2T/zBLtKbZynYfabJnL5lBj/JDJU4CHge0TWuCuKWPH0US6US3G9F27OaDb B8mqvU3vdsAmEMt9dXkqf7fVZUvqewJmbstaFVitY/qSDSf6gpC+wGMUUTHdLiLxiH v7SBAZAPJSC8AZ3baoFZ3zEG1EteMkDqI3aCkFpY= Date: Wed, 1 Jul 2020 12:49:46 +0200 From: Wolfram Sang To: Alain Volmat Subject: Re: [PATCH v2 1/4] i2c: smbus: add core function handling SMBus host-notify Message-ID: <20200701104946.GH2261@ninjato> References: <1593070769-9106-1-git-send-email-alain.volmat@st.com> <1593070769-9106-2-git-send-email-alain.volmat@st.com> MIME-Version: 1.0 In-Reply-To: <1593070769-9106-2-git-send-email-alain.volmat@st.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200701_064950_208490_EE577132 X-CRM114-Status: GOOD ( 28.68 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, alexandre.torgue@st.com, linux-kernel@vger.kernel.org, pierre-yves.mordret@st.com, robh+dt@kernel.org, linux-i2c@vger.kernel.org, mcoquelin.stm32@gmail.com, fabrice.gasnier@st.com, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org Content-Type: multipart/mixed; boundary="===============8455819347848934112==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============8455819347848934112== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="XIiC+We3v3zHqZ6Z" Content-Disposition: inline --XIiC+We3v3zHqZ6Z Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 25, 2020 at 09:39:26AM +0200, Alain Volmat wrote: > SMBus Host-Notify protocol, from the adapter point of view > consist of receiving a message from a client, including the > client address and some other data. >=20 > It can be simply handled by creating a new slave device > and registering a callback performing the parsing of the > message received from the client. >=20 > This commit introduces two new core functions > * i2c_new_smbus_host_notify_device > * i2c_free_smbus_host_notify_device > that take care of registration of the new slave device and > callback and will call i2c_handle_smbus_host_notify once a > Host-Notify event is received. >=20 > Signed-off-by: Alain Volmat > --- > v2: remove useless dev_err message in case of hnotify handling error > prevent handling hnotify in case of a incomplete write Okay, now I got it to work, I also noted a few more issues. First, I'd suggest s/i2c_smbus_host_notify/i2c_slave_host_notify/g for all occurences in this patch. This makes a stronger distinction between the generic HostNotify support and the slave specific one. Also, I wonder if this shouldn't go to i2c-smbus.c instead but I haven't checked if we end up in dependency hell then. Second best thought: at least move to i2c-core-slave.c, then we could save the #ifdeffery in the c-file? >=20 > drivers/i2c/i2c-core-smbus.c | 110 +++++++++++++++++++++++++++++++++++++= ++++++ > include/linux/i2c-smbus.h | 2 + > 2 files changed, 112 insertions(+) >=20 > diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c > index 56bb840142e3..3a37664fb5f6 100644 > --- a/drivers/i2c/i2c-core-smbus.c > +++ b/drivers/i2c/i2c-core-smbus.c > @@ -708,3 +708,113 @@ int of_i2c_setup_smbus_alert(struct i2c_adapter *ad= apter) > } > EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert); > #endif > + > +#if IS_ENABLED(CONFIG_I2C_SLAVE) > +struct i2c_smbus_host_notify_status { > + bool notify_start; > + u8 addr; > +}; > + > +static int i2c_smbus_host_notify_cb(struct i2c_client *client, > + enum i2c_slave_event event, u8 *val) > +{ > + struct i2c_smbus_host_notify_status *status =3D client->dev.platform_da= ta; > + int ret; > + > + switch (event) { > + case I2C_SLAVE_WRITE_REQUESTED: > + status->notify_start =3D true; > + break; > + case I2C_SLAVE_WRITE_RECEIVED: > + /* We only retrieve the first byte received (addr) > + * since there is currently no support to retrieve the data > + * parameter from the client. > + */ > + if (!status->notify_start) > + break; > + status->addr =3D *val; > + status->notify_start =3D false; So, we are safe if the message is too short. Otherwise, we capture the first byte (=3D=3D address) only, right. Further bytes until STOP are discarded. So, we don't check if the message is too long and contains more than the status word. Maybe we should add that? > + break; > + case I2C_SLAVE_STOP: > + /* In case of incomplete write, don't handle host-notify */ > + if (status->notify_start) { > + status->notify_start =3D false; > + break; > + } > + > + ret =3D i2c_handle_smbus_host_notify(client->adapter, > + status->addr); > + if (ret < 0) > + return ret; > + break; > + default: The missing cases are mandatory. From my testunit driver: case I2C_SLAVE_READ_REQUESTED: case I2C_SLAVE_READ_PROCESSED: *val =3D 0xff; break; > + /* Only handle necessary events */ > + break; > + } > + > + return 0; > +} > + =2E.. > --- a/include/linux/i2c-smbus.h > +++ b/include/linux/i2c-smbus.h > @@ -38,6 +38,8 @@ static inline int of_i2c_setup_smbus_alert(struct i2c_a= dapter *adap) > return 0; > } > #endif > +struct i2c_client *i2c_new_smbus_host_notify_device(struct i2c_adapter *= adapter); > +void i2c_free_smbus_host_notify_device(struct i2c_client *client); Those need to be guarded with I2C_SLAVE as well. And an #else branch with empty/successful placeholders. > =20 > #if IS_ENABLED(CONFIG_I2C_SMBUS) && IS_ENABLED(CONFIG_DMI) > void i2c_register_spd(struct i2c_adapter *adap); > --=20 > 2.7.4 >=20 --XIiC+We3v3zHqZ6Z Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAl78akUACgkQFA3kzBSg KbbHlRAAs1l3c2tSTqVOJVVbdpNSWj9cd9fgOQ5e7DrFZklEo+fWZusrPMYkSk9r q8HIUFiiAiEACt4r46NMdLoJguvcXhm5Hqlpi3B7EDAgIH6/eieb2MsFFkwd8Lh3 HzdfVYY9fgsYmdpoXQlu+eXI7Q3U3kWaWfhFzWnTdOKgTSdflSDDpV+RJpqnXYAb j53W1o4RZTxH+PlyQJA+1kxwb4ij4XTE26KpWDeI4yknN0vFx+IqHJzwaF1niYzU 6WsrXRBMmLOcBOdG0OQQFWCWH7YPORqqiUx7G9sgf21cZal/N3XfuVZdIZzgpaFi qCvcLckqrt7E0rE8UrJQwQigZMlgQNn5KARaux6ZisiCbZ5ncLzd91DBByxh53Au 0FwKdnPWmmbU3OhFStt7JFXj64Q7kp28GPkASuW5fiGF0tpxBvDVtUYmKSx755qJ 1Z7vRyTR0FpiqOEiUov3bkRIHtoJP6NIzD8LBcJBYjRAV/zKsDqDIeZmRRT6XUsf SxQKFTriO1xEXUXVnOI3YJrXwcyveTRxvq81LbDnDUZJ2wxhPVWMkUiDj80LJdm7 Yzi53x0nT9AtzDgRmYRVglLIc+/ZjD9h++NpVivZD8bCOrjOb1SCDks4hPOyaUqa zWx9IDDq1g2VpATy+Erga6U6cSbrD5XxccOsgjunhoOdZLp8ZGE= =ODPH -----END PGP SIGNATURE----- --XIiC+We3v3zHqZ6Z-- --===============8455819347848934112== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============8455819347848934112==--