From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 78C2A24B2B; Mon, 4 Mar 2024 10:00:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709546451; cv=none; b=l0MT5Bsw8eczFjqsaGmUMf5MBFplnBxuno1zfVakksakhQjqlSFbo/fZcvFGt8aNIzuKTgBv20DdnPBcDYtjJT7gp7WZkDOwziZHZMU7+HiKwezUdNEoDEH00fE1CIPaYlr3uvifetFSKndXRLb7ubu+rsAgcMV0VUOd65Oz7t0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709546451; c=relaxed/simple; bh=Ltz90TO6kzYU5NNZDHlenUvdrRAeb9Z4YTkBpwDlM6k=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:To:From:Subject: References:In-Reply-To; b=flHKRM4ThWRKTv0OM7OnOduDta4uLQ23yuEzktjmGbirnDdkNqSlvu93BFRVHOspDzqXhf5agjf5KEJv2N6JlFbBYOBY10X53FvQdo1Bw+WtOT4ZpzMmBFOZieZioh9sG0jxUw+45nVSD9zWXhPpTyonGwLdbhRRVBzbUWNxiJk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=maGBN6NO; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="maGBN6NO" Received: by mail.gandi.net (Postfix) with ESMTPSA id 35D9C240011; Mon, 4 Mar 2024 10:00:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1709546446; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pz1yibsIsNHyK2XZsyjOxrFSmLQ7xgeVb+gsJobLKuE=; b=maGBN6NOtE6vKHL1RnDS5WOvUaS37rhoc/gAC3aGOI2TqC8PyRPfBQq4vQxP0Gyyl2s4ax NviTY9rFf8juUhThZVE4NqOCS0a8GhBFxTqjJTWIT0Qj92967tm4lPeJtTyWmQd84EBBiy qldXFnyRJGr0n3OuBAJuYV7jdzwjN9j4fa4AG3ajgES9yK3hpFDGwv1AjLl3lXErJFMoxG 7PuFTSOxSFKsoOpmefpFyzU8wNLVcdJdgZE3F/4Q90L28+Q9T6+qgs8fZ0FGyFNNGffDEW B8V6f6p/JiNEsog8hUpRWKfVtcsdad7apTXg6PhzhUHOJ8dM2k1+VK0+1p+3HA== Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 04 Mar 2024 11:00:45 +0100 Message-Id: Cc: "Linus Walleij" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Thomas Bogendoerfer" , , , , , , "Gregory Clement" , "Vladimir Kondratiev" , "Thomas Petazzoni" , "Tawfik Bayouk" To: "Andi Shyti" From: =?utf-8?q?Th=C3=A9o_Lebrun?= Subject: Re: [PATCH v2 05/11] i2c: nomadik: use bitops helpers X-Mailer: aerc 0.15.2 References: <20240229-mbly-i2c-v2-0-b32ed18c098c@bootlin.com> <20240229-mbly-i2c-v2-5-b32ed18c098c@bootlin.com> <3kooaexx6vhlfwoojcpmnyhagupqwppwenjh4k7ucxbvlfpjn6@e3b7c3ocu6kc> In-Reply-To: <3kooaexx6vhlfwoojcpmnyhagupqwppwenjh4k7ucxbvlfpjn6@e3b7c3ocu6kc> X-GND-Sasl: theo.lebrun@bootlin.com Hello, On Sat Mar 2, 2024 at 2:31 AM CET, Andi Shyti wrote: [...] > > @@ -284,7 +290,7 @@ static int init_hw(struct nmk_i2c_dev *priv) > > } > > =20 > > /* enable peripheral, master mode operation */ > > -#define DEFAULT_I2C_REG_CR ((1 << 1) | I2C_CR_PE) > > +#define DEFAULT_I2C_REG_CR (FIELD_PREP(I2C_CR_OM, 0b01) | I2C_CR_PE) > > 0b01? OM is a two-bit field. We want to put 0b01 in that. We do not declare constants for its 4 potential values. Values are: - 0b00 slave mode - 0b01 master mode - 0b10 master/slave mode - 0b11 reserved To me the comment above DEFAULT_I2C_REG_CR is enough to show that we go into master mode. We could declare all values as constants but only 0b01 would get used. > > > /** > > * load_i2c_mcr_reg() - load the MCR register > > @@ -296,41 +302,42 @@ static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *p= riv, u16 flags) > > u32 mcr =3D 0; > > unsigned short slave_adr_3msb_bits; > > =20 > > - mcr |=3D GEN_MASK(priv->cli.slave_adr, I2C_MCR_A7, 1); > > + mcr |=3D FIELD_PREP(I2C_MCR_A7, priv->cli.slave_adr); > > =20 > > if (unlikely(flags & I2C_M_TEN)) { > > /* 10-bit address transaction */ > > - mcr |=3D GEN_MASK(2, I2C_MCR_AM, 12); > > + mcr |=3D FIELD_PREP(I2C_MCR_AM, 2); > > /* > > * Get the top 3 bits. > > * EA10 represents extended address in MCR. This includes > > * the extension (MSB bits) of the 7 bit address loaded > > * in A7 > > */ > > - slave_adr_3msb_bits =3D (priv->cli.slave_adr >> 7) & 0x7; > > + slave_adr_3msb_bits =3D FIELD_GET(GENMASK(9, 7), > > + priv->cli.slave_adr); > > This looks like the only one not having a define. Shall we give a > definition to GENMASK(9, 7)? Indeed. What should it be named? It could be generic; this is about getting the upper 3 bits from an extended (10-bit) I2C address? > > - mcr |=3D GEN_MASK(slave_adr_3msb_bits, I2C_MCR_EA10, 8); > > + mcr |=3D FIELD_PREP(I2C_MCR_EA10, slave_adr_3msb_bits); > > ... > > > @@ -824,15 +827,16 @@ static irqreturn_t i2c_irq_handler(int irq, void = *arg) > > * during the transaction. > > */ > > case I2C_IT_BERR: > > + { > > + u32 sr =3D readl(priv->virtbase + I2C_SR); > > please give a blank line after the variable definition. > > Besides, I'd prefer the assignment, when it is a bit more > complex, in a different line, as well. Will do. > Rest looks OK, didn't see anything off. Thanks for the review Andi! -- Th=C3=A9o Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D6C01C5478C for ; Mon, 4 Mar 2024 10:29:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:References:Subject:From:To: Cc:Message-Id:Date:Mime-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wsyfeESx4LtmOUvhbNmUCqDHl+4cMteachbYvr4ihN8=; b=cfBreFQtRsTU8P nHLtB9Y+aE1sV7G0K18Zgx6xbCEwhlAEXp0r4FsmocDHAre0I502SNb7fVoaKnXa2iRc+VEfPtfNc nxIGXCsGrXO/qtu0Cx8Hqx9NC7diiEE+zz41RZpMdc84R/wDnJMj/d0gq1NWPc12N0i8JF3FsAKVx a0xVIw2P5mh4pp3cbt4jz++RQXlNy8XSwByyJtizLl91wNIVUzERNxJ/c2WuMfFQr2nzcKJ9NA9md Ve8RM05Lsd3aRGHJxqpAFFNoVB08w5nhwIX2tZDE1VG0gAhEhWG9GRZ5xR3zlRXzavW6Zsy4pI+6u 8Jh3OTkHGSSZhvLv1R4w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rh5Zg-00000008bvA-0rmS; Mon, 04 Mar 2024 10:29:36 +0000 Received: from relay1-d.mail.gandi.net ([2001:4b98:dc4:8::221]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rh57s-00000008WYE-0DKa for linux-arm-kernel@lists.infradead.org; Mon, 04 Mar 2024 10:00:56 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 35D9C240011; Mon, 4 Mar 2024 10:00:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1709546446; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pz1yibsIsNHyK2XZsyjOxrFSmLQ7xgeVb+gsJobLKuE=; b=maGBN6NOtE6vKHL1RnDS5WOvUaS37rhoc/gAC3aGOI2TqC8PyRPfBQq4vQxP0Gyyl2s4ax NviTY9rFf8juUhThZVE4NqOCS0a8GhBFxTqjJTWIT0Qj92967tm4lPeJtTyWmQd84EBBiy qldXFnyRJGr0n3OuBAJuYV7jdzwjN9j4fa4AG3ajgES9yK3hpFDGwv1AjLl3lXErJFMoxG 7PuFTSOxSFKsoOpmefpFyzU8wNLVcdJdgZE3F/4Q90L28+Q9T6+qgs8fZ0FGyFNNGffDEW B8V6f6p/JiNEsog8hUpRWKfVtcsdad7apTXg6PhzhUHOJ8dM2k1+VK0+1p+3HA== Mime-Version: 1.0 Date: Mon, 04 Mar 2024 11:00:45 +0100 Message-Id: Cc: "Linus Walleij" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Thomas Bogendoerfer" , , , , , , "Gregory Clement" , "Vladimir Kondratiev" , "Thomas Petazzoni" , "Tawfik Bayouk" To: "Andi Shyti" From: =?utf-8?q?Th=C3=A9o_Lebrun?= Subject: Re: [PATCH v2 05/11] i2c: nomadik: use bitops helpers X-Mailer: aerc 0.15.2 References: <20240229-mbly-i2c-v2-0-b32ed18c098c@bootlin.com> <20240229-mbly-i2c-v2-5-b32ed18c098c@bootlin.com> <3kooaexx6vhlfwoojcpmnyhagupqwppwenjh4k7ucxbvlfpjn6@e3b7c3ocu6kc> In-Reply-To: <3kooaexx6vhlfwoojcpmnyhagupqwppwenjh4k7ucxbvlfpjn6@e3b7c3ocu6kc> X-GND-Sasl: theo.lebrun@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240304_020052_586800_80F77D07 X-CRM114-Status: GOOD ( 20.27 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org SGVsbG8sCgpPbiBTYXQgTWFyIDIsIDIwMjQgYXQgMjozMSBBTSBDRVQsIEFuZGkgU2h5dGkgd3Jv dGU6CgpbLi4uXQoKPiA+IEBAIC0yODQsNyArMjkwLDcgQEAgc3RhdGljIGludCBpbml0X2h3KHN0 cnVjdCBubWtfaTJjX2RldiAqcHJpdikKPiA+ICB9Cj4gPiAgCj4gPiAgLyogZW5hYmxlIHBlcmlw aGVyYWwsIG1hc3RlciBtb2RlIG9wZXJhdGlvbiAqLwo+ID4gLSNkZWZpbmUgREVGQVVMVF9JMkNf UkVHX0NSCSgoMSA8PCAxKSB8IEkyQ19DUl9QRSkKPiA+ICsjZGVmaW5lIERFRkFVTFRfSTJDX1JF R19DUgkoRklFTERfUFJFUChJMkNfQ1JfT00sIDBiMDEpIHwgSTJDX0NSX1BFKQo+Cj4gMGIwMT8K Ck9NIGlzIGEgdHdvLWJpdCBmaWVsZC4gV2Ugd2FudCB0byBwdXQgMGIwMSBpbiB0aGF0LiBXZSBk byBub3QgZGVjbGFyZQpjb25zdGFudHMgZm9yIGl0cyA0IHBvdGVudGlhbCB2YWx1ZXMuIFZhbHVl cyBhcmU6CgogLSAwYjAwIHNsYXZlIG1vZGUKIC0gMGIwMSBtYXN0ZXIgbW9kZQogLSAwYjEwIG1h c3Rlci9zbGF2ZSBtb2RlCiAtIDBiMTEgcmVzZXJ2ZWQKClRvIG1lIHRoZSBjb21tZW50IGFib3Zl IERFRkFVTFRfSTJDX1JFR19DUiBpcyBlbm91Z2ggdG8gc2hvdyB0aGF0IHdlIGdvCmludG8gbWFz dGVyIG1vZGUuIFdlIGNvdWxkIGRlY2xhcmUgYWxsIHZhbHVlcyBhcyBjb25zdGFudHMgYnV0IG9u bHkKMGIwMSB3b3VsZCBnZXQgdXNlZC4KCj4KPiA+ICAvKioKPiA+ICAgKiBsb2FkX2kyY19tY3Jf cmVnKCkgLSBsb2FkIHRoZSBNQ1IgcmVnaXN0ZXIKPiA+IEBAIC0yOTYsNDEgKzMwMiw0MiBAQCBz dGF0aWMgdTMyIGxvYWRfaTJjX21jcl9yZWcoc3RydWN0IG5ta19pMmNfZGV2ICpwcml2LCB1MTYg ZmxhZ3MpCj4gPiAgCXUzMiBtY3IgPSAwOwo+ID4gIAl1bnNpZ25lZCBzaG9ydCBzbGF2ZV9hZHJf M21zYl9iaXRzOwo+ID4gIAo+ID4gLQltY3IgfD0gR0VOX01BU0socHJpdi0+Y2xpLnNsYXZlX2Fk ciwgSTJDX01DUl9BNywgMSk7Cj4gPiArCW1jciB8PSBGSUVMRF9QUkVQKEkyQ19NQ1JfQTcsIHBy aXYtPmNsaS5zbGF2ZV9hZHIpOwo+ID4gIAo+ID4gIAlpZiAodW5saWtlbHkoZmxhZ3MgJiBJMkNf TV9URU4pKSB7Cj4gPiAgCQkvKiAxMC1iaXQgYWRkcmVzcyB0cmFuc2FjdGlvbiAqLwo+ID4gLQkJ bWNyIHw9IEdFTl9NQVNLKDIsIEkyQ19NQ1JfQU0sIDEyKTsKPiA+ICsJCW1jciB8PSBGSUVMRF9Q UkVQKEkyQ19NQ1JfQU0sIDIpOwo+ID4gIAkJLyoKPiA+ICAJCSAqIEdldCB0aGUgdG9wIDMgYml0 cy4KPiA+ICAJCSAqIEVBMTAgcmVwcmVzZW50cyBleHRlbmRlZCBhZGRyZXNzIGluIE1DUi4gVGhp cyBpbmNsdWRlcwo+ID4gIAkJICogdGhlIGV4dGVuc2lvbiAoTVNCIGJpdHMpIG9mIHRoZSA3IGJp dCBhZGRyZXNzIGxvYWRlZAo+ID4gIAkJICogaW4gQTcKPiA+ICAJCSAqLwo+ID4gLQkJc2xhdmVf YWRyXzNtc2JfYml0cyA9IChwcml2LT5jbGkuc2xhdmVfYWRyID4+IDcpICYgMHg3Owo+ID4gKwkJ c2xhdmVfYWRyXzNtc2JfYml0cyA9IEZJRUxEX0dFVChHRU5NQVNLKDksIDcpLAo+ID4gKwkJCQkJ CXByaXYtPmNsaS5zbGF2ZV9hZHIpOwo+Cj4gVGhpcyBsb29rcyBsaWtlIHRoZSBvbmx5IG9uZSBu b3QgaGF2aW5nIGEgZGVmaW5lLiBTaGFsbCB3ZSBnaXZlIGEKPiBkZWZpbml0aW9uIHRvIEdFTk1B U0soOSwgNyk/CgpJbmRlZWQuIFdoYXQgc2hvdWxkIGl0IGJlIG5hbWVkPyBJdCBjb3VsZCBiZSBn ZW5lcmljOyB0aGlzIGlzIGFib3V0CmdldHRpbmcgdGhlIHVwcGVyIDMgYml0cyBmcm9tIGFuIGV4 dGVuZGVkICgxMC1iaXQpIEkyQyBhZGRyZXNzPwoKPiA+IC0JCW1jciB8PSBHRU5fTUFTSyhzbGF2 ZV9hZHJfM21zYl9iaXRzLCBJMkNfTUNSX0VBMTAsIDgpOwo+ID4gKwkJbWNyIHw9IEZJRUxEX1BS RVAoSTJDX01DUl9FQTEwLCBzbGF2ZV9hZHJfM21zYl9iaXRzKTsKPgo+IC4uLgo+Cj4gPiBAQCAt ODI0LDE1ICs4MjcsMTYgQEAgc3RhdGljIGlycXJldHVybl90IGkyY19pcnFfaGFuZGxlcihpbnQg aXJxLCB2b2lkICphcmcpCj4gPiAgCSAqIGR1cmluZyB0aGUgdHJhbnNhY3Rpb24uCj4gPiAgCSAq Lwo+ID4gIAljYXNlIEkyQ19JVF9CRVJSOgo+ID4gKwl7Cj4gPiArCQl1MzIgc3IgPSByZWFkbChw cml2LT52aXJ0YmFzZSArIEkyQ19TUik7Cj4KPiBwbGVhc2UgZ2l2ZSBhIGJsYW5rIGxpbmUgYWZ0 ZXIgdGhlIHZhcmlhYmxlIGRlZmluaXRpb24uCj4KPiBCZXNpZGVzLCBJJ2QgcHJlZmVyIHRoZSBh c3NpZ25tZW50LCB3aGVuIGl0IGlzIGEgYml0IG1vcmUKPiBjb21wbGV4LCBpbiBhIGRpZmZlcmVu dCBsaW5lLCBhcyB3ZWxsLgoKV2lsbCBkby4KCj4gUmVzdCBsb29rcyBPSywgZGlkbid0IHNlZSBh bnl0aGluZyBvZmYuCgpUaGFua3MgZm9yIHRoZSByZXZpZXcgQW5kaSEKCi0tClRow6lvIExlYnJ1 biwgQm9vdGxpbgpFbWJlZGRlZCBMaW51eCBhbmQgS2VybmVsIGVuZ2luZWVyaW5nCmh0dHBzOi8v Ym9vdGxpbi5jb20KCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3Rz LmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5m by9saW51eC1hcm0ta2VybmVsCg==