[PATCH 0/2] regmap: provide simple bitops and use them in a driver

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/2] regmap: provide simple bitops and use them in a driver
@ 2020-05-28 12:34 Bartosz Golaszewski
  2020-05-28 12:34 ` [PATCH 1/2] regmap: provide helpers for simple bit operations Bartosz Golaszewski
  2020-05-28 12:34 ` [PATCH 2/2] net: ethernet: mtk-star-emac: use regmap bitops Bartosz Golaszewski
  0 siblings, 2 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2020-05-28 12:34 UTC (permalink / raw)
  To: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, Mark Brown
  Cc: netdev, linux-arm-kernel, linux-mediatek, linux-kernel,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Hi Mark,

I noticed that oftentimes I use regmap_update_bits() for simple bit
setting or clearing. In this case the fourth argument is superfluous as
it's always 0 or equal to the mask argument.

This series proposes to add simple bit operations for setting, clearing
and testing specific bits with regmap.

The second patch uses all three in a driver that got recently picked into
the net-next tree.

The patches obviously target different trees so - if you're ok with
the change itself - I propose you pick the first one into your regmap
tree for v5.8 and then I'll resend the second patch to add the first
user for these macros for v5.9.

Bartosz Golaszewski (2):
  regmap: provide helpers for simple bit operations
  net: ethernet: mtk-star-emac: use regmap bitops

 drivers/net/ethernet/mediatek/mtk_star_emac.c | 80 ++++++++-----------
 include/linux/regmap.h                        | 18 +++++
 2 files changed, 53 insertions(+), 45 deletions(-)

-- 
2.25.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 12:34 [PATCH 0/2] regmap: provide simple bitops and use them in a driver Bartosz Golaszewski
@ 2020-05-28 12:34 ` Bartosz Golaszewski
  2020-05-28 13:29   ` Mark Brown
  2020-05-28 12:34 ` [PATCH 2/2] net: ethernet: mtk-star-emac: use regmap bitops Bartosz Golaszewski
  1 sibling, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2020-05-28 12:34 UTC (permalink / raw)
  To: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, Mark Brown
  Cc: netdev, linux-arm-kernel, linux-mediatek, linux-kernel,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

In many instances regmap_update_bits() is used for simple bit setting
and clearing. In these cases the last argument is redundant and we can
hide it with a macro.

This adds three new macros for simple bit operations: set_bits,
clear_bits and test_bits.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/regmap.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 40b07168fd8e..6ef829169f36 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -71,6 +71,24 @@ struct reg_sequence {
 	unsigned int delay_us;
 };
 
+#define regmap_set_bits(map, reg, bits) \
+	regmap_update_bits_base(map, reg, bits, bits, NULL, false, false)
+#define regmap_clear_bits(map, reg, bits) \
+	regmap_update_bits_base(map, reg, bits, 0, NULL, false, false)
+/*
+ * Returns -1 if the underlying regmap_read() fails, 0 if at least one of the
+ * tested bits is not set and 1 if all tested bits are set.
+ */
+#define regmap_test_bits(map, reg, bits) \
+({ \
+	unsigned int __val, __ret, __bits; \
+	__bits = (bits); \
+	__ret = regmap_read(map, reg, &__val); \
+	if (__ret == 0) \
+		__ret = (__val & __bits) == __bits ? 1 : 0; \
+	__ret; \
+})
+
 #define	regmap_update_bits(map, reg, mask, val) \
 	regmap_update_bits_base(map, reg, mask, val, NULL, false, false)
 #define	regmap_update_bits_async(map, reg, mask, val)\
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 12:34 ` [PATCH 1/2] regmap: provide helpers for simple bit operations Bartosz Golaszewski
@ 2020-05-28 13:29   ` Mark Brown
  2020-05-28 13:32     ` Bartosz Golaszewski
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2020-05-28 13:29 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, netdev, linux-arm-kernel,
	linux-mediatek, linux-kernel, Fabien Parent, Stephane Le Provost,
	Pedro Tsai, Andrew Perepech, Bartosz Golaszewski

[-- Attachment #1: Type: text/plain, Size: 200 bytes --]

On Thu, May 28, 2020 at 02:34:58PM +0200, Bartosz Golaszewski wrote:

> This adds three new macros for simple bit operations: set_bits,
> clear_bits and test_bits.

Why macros and not static inlines?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 13:29   ` Mark Brown
@ 2020-05-28 13:32     ` Bartosz Golaszewski
  2020-05-28 13:48       ` Mark Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2020-05-28 13:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Linux Kernel Mailing List,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

czw., 28 maj 2020 o 15:29 Mark Brown <broonie@kernel.org> napisał(a):
>
> On Thu, May 28, 2020 at 02:34:58PM +0200, Bartosz Golaszewski wrote:
>
> > This adds three new macros for simple bit operations: set_bits,
> > clear_bits and test_bits.
>
> Why macros and not static inlines?

The existing regmap_update_bits_*() helpers are macros too, so I tried
to stay consistent. Any reason why they are macros and not static
inlines? If there's none, then why not convert them too? Otherwise
we'd have a static inline expanding a macro which in turn is calling a
function (regmap_update_bits_base()).

Bartosz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 13:32     ` Bartosz Golaszewski
@ 2020-05-28 13:48       ` Mark Brown
  2020-05-28 13:57         ` Bartosz Golaszewski
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Brown @ 2020-05-28 13:48 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Linux Kernel Mailing List,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

On Thu, May 28, 2020 at 03:32:40PM +0200, Bartosz Golaszewski wrote:
> czw., 28 maj 2020 o 15:29 Mark Brown <broonie@kernel.org> napisał(a):

> > Why macros and not static inlines?

> The existing regmap_update_bits_*() helpers are macros too, so I tried
> to stay consistent. Any reason why they are macros and not static
> inlines? If there's none, then why not convert them too? Otherwise
> we'd have a static inline expanding a macro which in turn is calling a
> function (regmap_update_bits_base()).

Not really, I think it was just that they're argument tables.  It'd be
good to convert them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 13:48       ` Mark Brown
@ 2020-05-28 13:57         ` Bartosz Golaszewski
  2020-05-28 13:58           ` Mark Brown
  0 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2020-05-28 13:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Linux Kernel Mailing List,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

czw., 28 maj 2020 o 15:48 Mark Brown <broonie@kernel.org> napisał(a):
>
> On Thu, May 28, 2020 at 03:32:40PM +0200, Bartosz Golaszewski wrote:
> > czw., 28 maj 2020 o 15:29 Mark Brown <broonie@kernel.org> napisał(a):
>
> > > Why macros and not static inlines?
>
> > The existing regmap_update_bits_*() helpers are macros too, so I tried
> > to stay consistent. Any reason why they are macros and not static
> > inlines? If there's none, then why not convert them too? Otherwise
> > we'd have a static inline expanding a macro which in turn is calling a
> > function (regmap_update_bits_base()).
>
> Not really, I think it was just that they're argument tables.  It'd be
> good to convert them.

Ok. So I'm seeing there are a lot of macros in regmap.h that could
become static inlines but given the amount of regmap users: how about
we do it separately and in the meantime I'll just modify this series
to use static inlines?

Bartosz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] regmap: provide helpers for simple bit operations
  2020-05-28 13:57         ` Bartosz Golaszewski
@ 2020-05-28 13:58           ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2020-05-28 13:58 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, netdev, Linux ARM,
	moderated list:ARM/Mediatek SoC..., Linux Kernel Mailing List,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

On Thu, May 28, 2020 at 03:57:24PM +0200, Bartosz Golaszewski wrote:

> Ok. So I'm seeing there are a lot of macros in regmap.h that could
> become static inlines but given the amount of regmap users: how about
> we do it separately and in the meantime I'll just modify this series
> to use static inlines?

Sure.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] net: ethernet: mtk-star-emac: use regmap bitops
  2020-05-28 12:34 [PATCH 0/2] regmap: provide simple bitops and use them in a driver Bartosz Golaszewski
  2020-05-28 12:34 ` [PATCH 1/2] regmap: provide helpers for simple bit operations Bartosz Golaszewski
@ 2020-05-28 12:34 ` Bartosz Golaszewski
  1 sibling, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2020-05-28 12:34 UTC (permalink / raw)
  To: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger, Mark Brown
  Cc: netdev, linux-arm-kernel, linux-mediatek, linux-kernel,
	Fabien Parent, Stephane Le Provost, Pedro Tsai, Andrew Perepech,
	Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Shrink the code visually by replacing regmap_update_bits() with
appropriate regmap bit operations where applicable.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/mediatek/mtk_star_emac.c | 80 ++++++++-----------
 1 file changed, 35 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/ethernet/mediatek/mtk_star_emac.c
index 8596ca0e60eb..326ac792a4a0 100644
--- a/drivers/net/ethernet/mediatek/mtk_star_emac.c
+++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c
@@ -413,8 +413,8 @@ static void mtk_star_dma_unmap_tx(struct mtk_star_priv *priv,
 
 static void mtk_star_nic_disable_pd(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_MAC_CFG,
-			   MTK_STAR_BIT_MAC_CFG_NIC_PD, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_MAC_CFG,
+			  MTK_STAR_BIT_MAC_CFG_NIC_PD);
 }
 
 /* Unmask the three interrupts we care about, mask all others. */
@@ -434,41 +434,38 @@ static void mtk_star_intr_disable(struct mtk_star_priv *priv)
 
 static void mtk_star_intr_enable_tx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_BIT_INT_STS_TNTC, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			  MTK_STAR_BIT_INT_STS_TNTC);
 }
 
 static void mtk_star_intr_enable_rx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_BIT_INT_STS_FNRC, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			  MTK_STAR_BIT_INT_STS_FNRC);
 }
 
 static void mtk_star_intr_enable_stats(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_REG_INT_STS_MIB_CNT_TH, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			  MTK_STAR_REG_INT_STS_MIB_CNT_TH);
 }
 
 static void mtk_star_intr_disable_tx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_BIT_INT_STS_TNTC,
-			   MTK_STAR_BIT_INT_STS_TNTC);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			MTK_STAR_BIT_INT_STS_TNTC);
 }
 
 static void mtk_star_intr_disable_rx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_BIT_INT_STS_FNRC,
-			   MTK_STAR_BIT_INT_STS_FNRC);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			MTK_STAR_BIT_INT_STS_FNRC);
 }
 
 static void mtk_star_intr_disable_stats(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_INT_MASK,
-			   MTK_STAR_REG_INT_STS_MIB_CNT_TH,
-			   MTK_STAR_REG_INT_STS_MIB_CNT_TH);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_INT_MASK,
+			MTK_STAR_REG_INT_STS_MIB_CNT_TH);
 }
 
 static unsigned int mtk_star_intr_read(struct mtk_star_priv *priv)
@@ -524,12 +521,10 @@ static void mtk_star_dma_init(struct mtk_star_priv *priv)
 
 static void mtk_star_dma_start(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_TX_DMA_CTRL,
-			   MTK_STAR_BIT_TX_DMA_CTRL_START,
-			   MTK_STAR_BIT_TX_DMA_CTRL_START);
-	regmap_update_bits(priv->regs, MTK_STAR_REG_RX_DMA_CTRL,
-			   MTK_STAR_BIT_RX_DMA_CTRL_START,
-			   MTK_STAR_BIT_RX_DMA_CTRL_START);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_TX_DMA_CTRL,
+			MTK_STAR_BIT_TX_DMA_CTRL_START);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_RX_DMA_CTRL,
+			MTK_STAR_BIT_RX_DMA_CTRL_START);
 }
 
 static void mtk_star_dma_stop(struct mtk_star_priv *priv)
@@ -553,16 +548,14 @@ static void mtk_star_dma_disable(struct mtk_star_priv *priv)
 
 static void mtk_star_dma_resume_rx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_RX_DMA_CTRL,
-			   MTK_STAR_BIT_RX_DMA_CTRL_RESUME,
-			   MTK_STAR_BIT_RX_DMA_CTRL_RESUME);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_RX_DMA_CTRL,
+			MTK_STAR_BIT_RX_DMA_CTRL_RESUME);
 }
 
 static void mtk_star_dma_resume_tx(struct mtk_star_priv *priv)
 {
-	regmap_update_bits(priv->regs, MTK_STAR_REG_TX_DMA_CTRL,
-			   MTK_STAR_BIT_TX_DMA_CTRL_RESUME,
-			   MTK_STAR_BIT_TX_DMA_CTRL_RESUME);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_TX_DMA_CTRL,
+			MTK_STAR_BIT_TX_DMA_CTRL_RESUME);
 }
 
 static void mtk_star_set_mac_addr(struct net_device *ndev)
@@ -845,8 +838,8 @@ static int mtk_star_hash_wait_ok(struct mtk_star_priv *priv)
 		return ret;
 
 	/* Check the BIST_OK bit. */
-	regmap_read(priv->regs, MTK_STAR_REG_HASH_CTRL, &val);
-	if (!(val & MTK_STAR_BIT_HASH_CTRL_BIST_OK))
+	if (!regmap_test_bits(priv->regs, MTK_STAR_REG_HASH_CTRL,
+			      MTK_STAR_BIT_HASH_CTRL_BIST_OK))
 		return -EIO;
 
 	return 0;
@@ -880,12 +873,10 @@ static int mtk_star_reset_hash_table(struct mtk_star_priv *priv)
 	if (ret)
 		return ret;
 
-	regmap_update_bits(priv->regs, MTK_STAR_REG_HASH_CTRL,
-			   MTK_STAR_BIT_HASH_CTRL_BIST_EN,
-			   MTK_STAR_BIT_HASH_CTRL_BIST_EN);
-	regmap_update_bits(priv->regs, MTK_STAR_REG_TEST1,
-			   MTK_STAR_BIT_TEST1_RST_HASH_MBIST,
-			   MTK_STAR_BIT_TEST1_RST_HASH_MBIST);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_HASH_CTRL,
+			MTK_STAR_BIT_HASH_CTRL_BIST_EN);
+	regmap_set_bits(priv->regs, MTK_STAR_REG_TEST1,
+			MTK_STAR_BIT_TEST1_RST_HASH_MBIST);
 
 	return mtk_star_hash_wait_ok(priv);
 }
@@ -1016,13 +1007,13 @@ static int mtk_star_enable(struct net_device *ndev)
 		return ret;
 
 	/* Setup the hashing algorithm */
-	regmap_update_bits(priv->regs, MTK_STAR_REG_ARL_CFG,
-			   MTK_STAR_BIT_ARL_CFG_HASH_ALG |
-			   MTK_STAR_BIT_ARL_CFG_MISC_MODE, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_ARL_CFG,
+			  MTK_STAR_BIT_ARL_CFG_HASH_ALG |
+			  MTK_STAR_BIT_ARL_CFG_MISC_MODE);
 
 	/* Don't strip VLAN tags */
-	regmap_update_bits(priv->regs, MTK_STAR_REG_MAC_CFG,
-			   MTK_STAR_BIT_MAC_CFG_VLAN_STRIP, 0);
+	regmap_clear_bits(priv->regs, MTK_STAR_REG_MAC_CFG,
+			  MTK_STAR_BIT_MAC_CFG_VLAN_STRIP);
 
 	/* Setup DMA */
 	mtk_star_dma_init(priv);
@@ -1204,9 +1195,8 @@ static void mtk_star_set_rx_mode(struct net_device *ndev)
 	int ret;
 
 	if (ndev->flags & IFF_PROMISC) {
-		regmap_update_bits(priv->regs, MTK_STAR_REG_ARL_CFG,
-				   MTK_STAR_BIT_ARL_CFG_MISC_MODE,
-				   MTK_STAR_BIT_ARL_CFG_MISC_MODE);
+		regmap_set_bits(priv->regs, MTK_STAR_REG_ARL_CFG,
+				MTK_STAR_BIT_ARL_CFG_MISC_MODE);
 	} else if (netdev_mc_count(ndev) > MTK_STAR_HASHTABLE_MC_LIMIT ||
 		   ndev->flags & IFF_ALLMULTI) {
 		for (i = 0; i < MTK_STAR_HASHTABLE_SIZE_MAX; i++) {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-28 13:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-28 12:34 [PATCH 0/2] regmap: provide simple bitops and use them in a driver Bartosz Golaszewski
2020-05-28 12:34 ` [PATCH 1/2] regmap: provide helpers for simple bit operations Bartosz Golaszewski
2020-05-28 13:29   ` Mark Brown
2020-05-28 13:32     ` Bartosz Golaszewski
2020-05-28 13:48       ` Mark Brown
2020-05-28 13:57         ` Bartosz Golaszewski
2020-05-28 13:58           ` Mark Brown
2020-05-28 12:34 ` [PATCH 2/2] net: ethernet: mtk-star-emac: use regmap bitops Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).