Message ID | 20220209182521.55632-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v1,1/1] media: c8sectpfe: Clean up handling of *_buffer_aligned | expand |
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?
+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?
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 > >
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?
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,
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(-)