[PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

There are a few cases where code is harder than needed to read.
Improve those by:
- dropping unnecessary castings (see note below)
- use PTR_ALING() to be more explicit on what's going on there
- use proper definitions instead of hard coded values

Note, dropping castings will allow to perform an additional check
that type is not changed from void * to something else, e.g. u64,
which may very well break the bitmap APIs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 .../platform/sti/c8sectpfe/c8sectpfe-core.c   | 30 +++++++------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 7bb1384e4bad..d60908ec9ea7 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -107,7 +107,7 @@ static void channel_swdemux_tsklet(struct tasklet_struct *t)
 				size,
 				DMA_FROM_DEVICE);
 
-	buf = (u8 *) channel->back_buffer_aligned;
+	buf = channel->back_buffer_aligned;
 
 	dev_dbg(fei->dev,
 		"chan=%d channel=%p num_packets = %d, buf = %p, pos = 0x%x\n\trp=0x%lx, wp=0x%lx\n",
@@ -176,7 +176,7 @@ static int c8sectpfe_start_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 	channel = fei->channel_data[stdemux->tsin_index];
 
-	bitmap = (unsigned long *) channel->pid_buffer_aligned;
+	bitmap = channel->pid_buffer_aligned;
 
 	/* 8192 is a special PID */
 	if (dvbdmxfeed->pid == 8192) {
@@ -272,7 +272,7 @@ static int c8sectpfe_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 	channel = fei->channel_data[stdemux->tsin_index];
 
-	bitmap = (unsigned long *) channel->pid_buffer_aligned;
+	bitmap = channel->pid_buffer_aligned;
 
 	if (dvbdmxfeed->pid == 8192) {
 		tmp = readl(fei->io + C8SECTPFE_IB_PID_SET(channel->tsin_id));
@@ -333,8 +333,7 @@ static int c8sectpfe_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 			__func__, __LINE__, stdemux, channel->tsin_id);
 
 		/* turn off all PIDS in the bitmap */
-		memset((void *)channel->pid_buffer_aligned
-			, 0x00, PID_TABLE_SIZE);
+		memset(channel->pid_buffer_aligned, 0, PID_TABLE_SIZE);
 
 		/* manage cache so data is visible to HW */
 		dma_sync_single_for_device(fei->dev,
@@ -458,23 +457,19 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 
 	init_completion(&tsin->idle_completion);
 
-	tsin->back_buffer_start = kzalloc(FEI_BUFFER_SIZE +
-					FEI_ALIGNMENT, GFP_KERNEL);
-
+	tsin->back_buffer_start = kzalloc(FEI_BUFFER_SIZE + FEI_ALIGNMENT, GFP_KERNEL);
 	if (!tsin->back_buffer_start) {
 		ret = -ENOMEM;
 		goto err_unmap;
 	}
 
 	/* Ensure backbuffer is 32byte aligned */
-	tsin->back_buffer_aligned = tsin->back_buffer_start
-		+ FEI_ALIGNMENT;
+	tsin->back_buffer_aligned = tsin->back_buffer_start + FEI_ALIGNMENT;
 
-	tsin->back_buffer_aligned = (void *)
-		(((uintptr_t) tsin->back_buffer_aligned) & ~0x1F);
+	tsin->back_buffer_aligned = PTR_ALIGN(tsin->back_buffer_aligned, FEI_ALIGNMENT);
 
 	tsin->back_buffer_busaddr = dma_map_single(fei->dev,
-					(void *)tsin->back_buffer_aligned,
+					tsin->back_buffer_aligned,
 					FEI_BUFFER_SIZE,
 					DMA_BIDIRECTIONAL);
 
@@ -489,8 +484,7 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 	 * per pid. By powers of deduction we conclude stih407 family
 	 * is configured (at SoC design stage) for bit per pid.
 	 */
-	tsin->pid_buffer_start = kzalloc(2048, GFP_KERNEL);
-
+	tsin->pid_buffer_start = kzalloc(PID_TABLE_SIZE + PID_TABLE_SIZE, GFP_KERNEL);
 	if (!tsin->pid_buffer_start) {
 		ret = -ENOMEM;
 		goto err_unmap;
@@ -503,11 +497,9 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 	 * the register.
 	 */
 
-	tsin->pid_buffer_aligned = tsin->pid_buffer_start +
-		PID_TABLE_SIZE;
+	tsin->pid_buffer_aligned = tsin->pid_buffer_start + PID_TABLE_SIZE;
 
-	tsin->pid_buffer_aligned = (void *)
-		(((uintptr_t) tsin->pid_buffer_aligned) & ~0x3ff);
+	tsin->pid_buffer_aligned = PTR_ALIGN(tsin->pid_buffer_aligned, PID_TABLE_SIZE);
 
 	tsin->pid_buffer_busaddr = dma_map_single(fei->dev,
 						tsin->pid_buffer_aligned,
-- 
2.34.1

[PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

There are a few cases where code is harder than needed to read.
Improve those by:
- dropping unnecessary castings (see note below)
- use PTR_ALING() to be more explicit on what's going on there
- use proper definitions instead of hard coded values

Note, dropping castings will allow to perform an additional check
that type is not changed from void * to something else, e.g. u64,
which may very well break the bitmap APIs.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 .../platform/sti/c8sectpfe/c8sectpfe-core.c   | 30 +++++++------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
index 7bb1384e4bad..d60908ec9ea7 100644
--- a/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
+++ b/drivers/media/platform/sti/c8sectpfe/c8sectpfe-core.c
@@ -107,7 +107,7 @@ static void channel_swdemux_tsklet(struct tasklet_struct *t)
 				size,
 				DMA_FROM_DEVICE);
 
-	buf = (u8 *) channel->back_buffer_aligned;
+	buf = channel->back_buffer_aligned;
 
 	dev_dbg(fei->dev,
 		"chan=%d channel=%p num_packets = %d, buf = %p, pos = 0x%x\n\trp=0x%lx, wp=0x%lx\n",
@@ -176,7 +176,7 @@ static int c8sectpfe_start_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 	channel = fei->channel_data[stdemux->tsin_index];
 
-	bitmap = (unsigned long *) channel->pid_buffer_aligned;
+	bitmap = channel->pid_buffer_aligned;
 
 	/* 8192 is a special PID */
 	if (dvbdmxfeed->pid == 8192) {
@@ -272,7 +272,7 @@ static int c8sectpfe_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 	channel = fei->channel_data[stdemux->tsin_index];
 
-	bitmap = (unsigned long *) channel->pid_buffer_aligned;
+	bitmap = channel->pid_buffer_aligned;
 
 	if (dvbdmxfeed->pid == 8192) {
 		tmp = readl(fei->io + C8SECTPFE_IB_PID_SET(channel->tsin_id));
@@ -333,8 +333,7 @@ static int c8sectpfe_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 			__func__, __LINE__, stdemux, channel->tsin_id);
 
 		/* turn off all PIDS in the bitmap */
-		memset((void *)channel->pid_buffer_aligned
-			, 0x00, PID_TABLE_SIZE);
+		memset(channel->pid_buffer_aligned, 0, PID_TABLE_SIZE);
 
 		/* manage cache so data is visible to HW */
 		dma_sync_single_for_device(fei->dev,
@@ -458,23 +457,19 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 
 	init_completion(&tsin->idle_completion);
 
-	tsin->back_buffer_start = kzalloc(FEI_BUFFER_SIZE +
-					FEI_ALIGNMENT, GFP_KERNEL);
-
+	tsin->back_buffer_start = kzalloc(FEI_BUFFER_SIZE + FEI_ALIGNMENT, GFP_KERNEL);
 	if (!tsin->back_buffer_start) {
 		ret = -ENOMEM;
 		goto err_unmap;
 	}
 
 	/* Ensure backbuffer is 32byte aligned */
-	tsin->back_buffer_aligned = tsin->back_buffer_start
-		+ FEI_ALIGNMENT;
+	tsin->back_buffer_aligned = tsin->back_buffer_start + FEI_ALIGNMENT;
 
-	tsin->back_buffer_aligned = (void *)
-		(((uintptr_t) tsin->back_buffer_aligned) & ~0x1F);
+	tsin->back_buffer_aligned = PTR_ALIGN(tsin->back_buffer_aligned, FEI_ALIGNMENT);
 
 	tsin->back_buffer_busaddr = dma_map_single(fei->dev,
-					(void *)tsin->back_buffer_aligned,
+					tsin->back_buffer_aligned,
 					FEI_BUFFER_SIZE,
 					DMA_BIDIRECTIONAL);
 
@@ -489,8 +484,7 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 	 * per pid. By powers of deduction we conclude stih407 family
 	 * is configured (at SoC design stage) for bit per pid.
 	 */
-	tsin->pid_buffer_start = kzalloc(2048, GFP_KERNEL);
-
+	tsin->pid_buffer_start = kzalloc(PID_TABLE_SIZE + PID_TABLE_SIZE, GFP_KERNEL);
 	if (!tsin->pid_buffer_start) {
 		ret = -ENOMEM;
 		goto err_unmap;
@@ -503,11 +497,9 @@ static int configure_memdma_and_inputblock(struct c8sectpfei *fei,
 	 * the register.
 	 */
 
-	tsin->pid_buffer_aligned = tsin->pid_buffer_start +
-		PID_TABLE_SIZE;
+	tsin->pid_buffer_aligned = tsin->pid_buffer_start + PID_TABLE_SIZE;
 
-	tsin->pid_buffer_aligned = (void *)
-		(((uintptr_t) tsin->pid_buffer_aligned) & ~0x3ff);
+	tsin->pid_buffer_aligned = PTR_ALIGN(tsin->pid_buffer_aligned, PID_TABLE_SIZE);
 
 	tsin->pid_buffer_busaddr = dma_map_single(fei->dev,
 						tsin->pid_buffer_aligned,
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> There are a few cases where code is harder than needed to read.
> Improve those by:
> - dropping unnecessary castings (see note below)
> - use PTR_ALING() to be more explicit on what's going on there
> - use proper definitions instead of hard coded values
> 
> Note, dropping castings will allow to perform an additional check
> that type is not changed from void * to something else, e.g. u64,
> which may very well break the bitmap APIs.

Any comments on it?
Can it be applied?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> There are a few cases where code is harder than needed to read.
> Improve those by:
> - dropping unnecessary castings (see note below)
> - use PTR_ALING() to be more explicit on what's going on there
> - use proper definitions instead of hard coded values
> 
> Note, dropping castings will allow to perform an additional check
> that type is not changed from void * to something else, e.g. u64,
> which may very well break the bitmap APIs.

Any comments on it?
Can it be applied?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

+Hans (see below).

On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > There are a few cases where code is harder than needed to read.
> > Improve those by:
> > - dropping unnecessary castings (see note below)
> > - use PTR_ALING() to be more explicit on what's going on there
> > - use proper definitions instead of hard coded values
> > 
> > Note, dropping castings will allow to perform an additional check
> > that type is not changed from void * to something else, e.g. u64,
> > which may very well break the bitmap APIs.
> 
> Any comments on it?
> Can it be applied?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

+Hans (see below).

On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > There are a few cases where code is harder than needed to read.
> > Improve those by:
> > - dropping unnecessary castings (see note below)
> > - use PTR_ALING() to be more explicit on what's going on there
> > - use proper definitions instead of hard coded values
> > 
> > Note, dropping castings will allow to perform an additional check
> > that type is not changed from void * to something else, e.g. u64,
> > which may very well break the bitmap APIs.
> 
> Any comments on it?
> Can it be applied?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Alain Volmat]

Hi Andy,

sorry for the delay.

Reviewed-by: Alain Volmat <alain.volmat@foss.st.com>

Regards,
Alain

On Tue, Mar 08, 2022 at 12:25:42PM +0200, Andy Shevchenko wrote:
> +Hans (see below).
> 
> On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > > There are a few cases where code is harder than needed to read.
> > > Improve those by:
> > > - dropping unnecessary castings (see note below)
> > > - use PTR_ALING() to be more explicit on what's going on there
> > > - use proper definitions instead of hard coded values
> > > 
> > > Note, dropping castings will allow to perform an additional check
> > > that type is not changed from void * to something else, e.g. u64,
> > > which may very well break the bitmap APIs.
> > 
> > Any comments on it?
> > Can it be applied?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Alain Volmat]

Hi Andy,

sorry for the delay.

Reviewed-by: Alain Volmat <alain.volmat@foss.st.com>

Regards,
Alain

On Tue, Mar 08, 2022 at 12:25:42PM +0200, Andy Shevchenko wrote:
> +Hans (see below).
> 
> On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > > There are a few cases where code is harder than needed to read.
> > > Improve those by:
> > > - dropping unnecessary castings (see note below)
> > > - use PTR_ALING() to be more explicit on what's going on there
> > > - use proper definitions instead of hard coded values
> > > 
> > > Note, dropping castings will allow to perform an additional check
> > > that type is not changed from void * to something else, e.g. u64,
> > > which may very well break the bitmap APIs.
> > 
> > Any comments on it?
> > Can it be applied?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

On Tue, Mar 08, 2022 at 06:10:05PM +0100, Alain Volmat wrote:
> Hi Andy,
> 
> sorry for the delay.
> 
> Reviewed-by: Alain Volmat <alain.volmat@foss.st.com>

Thanks!

Can it be applied now?

> On Tue, Mar 08, 2022 at 12:25:42PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> > > On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > > > There are a few cases where code is harder than needed to read.
> > > > Improve those by:
> > > > - dropping unnecessary castings (see note below)
> > > > - use PTR_ALING() to be more explicit on what's going on there
> > > > - use proper definitions instead of hard coded values
> > > > 
> > > > Note, dropping castings will allow to perform an additional check
> > > > that type is not changed from void * to something else, e.g. u64,
> > > > which may very well break the bitmap APIs.
> > > 
> > > Any comments on it?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned

[Andy Shevchenko]

On Tue, Mar 08, 2022 at 06:10:05PM +0100, Alain Volmat wrote:
> Hi Andy,
> 
> sorry for the delay.
> 
> Reviewed-by: Alain Volmat <alain.volmat@foss.st.com>

Thanks!

Can it be applied now?

> On Tue, Mar 08, 2022 at 12:25:42PM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 02, 2022 at 05:51:42PM +0200, Andy Shevchenko wrote:
> > > On Wed, Feb 09, 2022 at 08:25:21PM +0200, Andy Shevchenko wrote:
> > > > There are a few cases where code is harder than needed to read.
> > > > Improve those by:
> > > > - dropping unnecessary castings (see note below)
> > > > - use PTR_ALING() to be more explicit on what's going on there
> > > > - use proper definitions instead of hard coded values
> > > > 
> > > > Note, dropping castings will allow to perform an additional check
> > > > that type is not changed from void * to something else, e.g. u64,
> > > > which may very well break the bitmap APIs.
> > > 
> > > Any comments on it?

-- 
With Best Regards,
Andy Shevchenko