Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752693AbdFUIlm (ORCPT ); Wed, 21 Jun 2017 04:41:42 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:53046 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751148AbdFUIlk (ORCPT ); Wed, 21 Jun 2017 04:41:40 -0400 Date: Wed, 21 Jun 2017 10:41:28 +0200 From: Maxime Ripard To: Jonathan Liu Cc: David Airlie , Chen-Yu Tsai , linux-kernel , dri-devel , linux-arm-kernel , linux-sunxi Subject: Re: [PATCH v2] drm/sun4i: hdmi: Implement I2C adapter for A10s DDC bus Message-ID: <20170621084128.6745bs6irwmccbx6@flea.lan> References: <20170612055235.31019-1-net147@gmail.com> <20170613111505.afyh4iotxdz5kceh@flea.lan> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7se76w45725vuaf2" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4015 Lines: 118 --7se76w45725vuaf2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 13, 2017 at 09:53:31PM +1000, Jonathan Liu wrote: > >> --- /dev/null > >> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_i2c.c > >> @@ -0,0 +1,163 @@ > >> +/* > >> + * Copyright (C) 2017 Jonathan Liu > >> + * > >> + * Jonathan Liu > > > > Is it? >=20 > I could add you to the copyright since you did the old one. But the > implementation is different. > I intend to rework this I2C driver to use the FIFO more to avoid > splitting larger transfers > 16 bytes and do the transfer in a single > command. Would you like to be added to the copyright? You took some code that you didn't create, and added some more stuff. The copyright on the initial code remains, and it has nothing to do with whether the author wants it or not, (s)he should be mentionned, along with you of course. > >> + ? SUN4I_HDMI_DDC_CMD_IMPLICIT_READ > >> + : SUN4I_HDMI_DDC_CMD_IMPLICIT_WRITE, > >> + hdmi->base + SUN4I_HDMI_DDC_CMD_REG); > >> + > >> + reg =3D readl(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG); > >> + writel(reg | SUN4I_HDMI_DDC_CTRL_START_CMD, > >> + hdmi->base + SUN4I_HDMI_DDC_CTRL_REG); > >> + > >> + if (!(msg->flags & I2C_M_RD)) { > >> + for (i =3D 0; i < count; i++) { > >> + writeb(*msg->buf++, hdmi->base > >> + + SUN4I_HDMI_DDC_FIFO_DATA_REG); > > > > writesb? > I intend to rework the FIFO handling so will need to continue using write= b. Then you'll change it when you'll rework it. Before then, you can use writesb. > > > >> + --msg->len; > >> + } > >> + } > >> + > >> + if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_CTRL_REG, > >> + reg, > >> + !(reg & SUN4I_HDMI_DDC_CTRL_START_CMD), > >> + 100, 100000)) > >> + return -EIO; > >> + > >> + reg =3D readl(hdmi->base + SUN4I_HDMI_DDC_INTERRUPT_STATUS_REG); > >> + reg &=3D ~SUN4I_HDMI_DDC_INTERRUPT_STATUS_FIFO_REQUEST; > I want to check all the other bits that are set if there are errors. Same thing here: you'll change it when that happens. > > > > You don't need to use that mask. > > > >> + if (reg !=3D SUN4I_HDMI_DDC_INTERRUPT_STATUS_TRANSFER_COMPLETE) > >> + return -EIO; > > > > !(reg & SUN4I_HDMI_DDC_INTERRUPT_STATUS_TRANSFER_COMPLETE) would be eno= ugh. > I want to check all the other bits that are set if there are errors. Same thing here. > > > >> + > >> + if (msg->flags & I2C_M_RD) { > >> + for (i =3D 0; i < count; i++) { > >> + *msg->buf++ =3D readb(hdmi->base > >> + + SUN4I_HDMI_DDC_FIFO_DATA_R= EG); > > > > readsb ? > I am reworking the FIFO handling so I will need to continue to use readb. Same thing here. Maxime --=20 Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com --7se76w45725vuaf2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBAgAGBQJZSjE4AAoJEBx+YmzsjxAgx0wQAKqRyx6jVjTOhYmJqdIkMSfy d8j9yNLMjempkYjhKupgsURJr6y6qmbSBKhQk7zsHQaJ4T6dmUo0ci0LtwhUw0JO yEvVyLTScDVvuiV+1w9WTHe2VfOvL2iXf9CTJqJV/Xc18+3qgtuZ0uVREsoVVKU1 v6izIyDUKHFKAmDGqwAn+iAMZfy3a3vWITb+JAaSRRK/mIq8vgZS7/6jICvvFUE3 WhnN0Lk/QzbNoSs48OWbFfJTmi1IlW76VpwY2vebIa3gWN+l93PXschBqsY9R98U 6zqUOXTDtIEQvr6g1fphVfWzjgZc3jcMX/zn94hdxPG5xkZ9ctKZOXRwlbhrDUIP PAGQP20qAMPUunZdidJdopl/xFR5LorLj53oYKe17bABK48T11tVQVp7JRif8ad1 ORpyRV4oEV3QPIvwH3L+WJNh1jGhOq/4H+XOSDJ627WI5+pOFiFNseQQ9w4vlQ8Z irYb2ao4onT/b+BZlQZ8X1X2YrPbv/I8q4gJ2N0+t7Vx4yhNZQsI8VujkSFpOFej 3fho2+U9mF77kDGy7iHTQjej9WAPJZZj6Fg0RXVS2/FNC4y0dR5s20r7W+zsxugE R4gSgU9Cor6VVY7Iw1nJ4enH3u1oDLtY8E5E+F5kcNBC1SO1bwtL9dl5WtobtPHK XLoIB+RRYuor+ntDoldW =5uto -----END PGP SIGNATURE----- --7se76w45725vuaf2--