| Message ID | 1394017710-671-1-git-send-email-sw0312.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5 March 2014 16:38, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: > There was assignment of memory bank with dma address converted > from physical address. But allocation has been changed with dma > function, so the assignment is not necessary. > > Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com> > --- > change from v1 > - fixes subject and adds proper description > --- > drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > index 2475a3c..ee05f2d 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c > @@ -44,8 +44,6 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev) > return -ENOMEM; > } > > - dev->bank1 = dev->bank1; Are you sure this isn't some kind of typo? If not then your commit description is too verbose to actually say that the code is redundant and could be removed. The code here is something like a = a; which does not make sense nor add any value and hence redundant and could be removed.
Hello Sachin, On 2014? 03? 05? 20:42, Sachin Kamat wrote: > On 5 March 2014 16:38, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: (...) >> - dev->bank1 = dev->bank1; > > Are you sure this isn't some kind of typo? If not then your commit > description is too verbose > to actually say that the code is redundant and could be removed. The > code here is something like > > a = a; > > which does not make sense nor add any value and hence redundant and > could be removed. Right, this meaningless code can be simply removed as like the first version. Anyway this redundant made from change of address type in earlier patch. So I tried to describe that. Regards, - Seung-Woo Kim
Hi Seung-Woo, On 6 March 2014 07:43, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: > Hello Sachin, > > On 2014? 03? 05? 20:42, Sachin Kamat wrote: >> On 5 March 2014 16:38, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: > > (...) > >>> - dev->bank1 = dev->bank1; >> >> Are you sure this isn't some kind of typo? If not then your commit >> description is too verbose >> to actually say that the code is redundant and could be removed. The >> code here is something like >> >> a = a; >> >> which does not make sense nor add any value and hence redundant and >> could be removed. > > Right, this meaningless code can be simply removed as like the first > version. Anyway this redundant made from change of address type in > earlier patch. So I tried to describe that. What Kamil meant was that it is not a good practice to leave the commit description blank however trivial the patch might be. So a single line stating the obvious should be sufficient in this case.
Hello Sachin, On 2014? 03? 06? 12:28, Sachin Kamat wrote: > Hi Seung-Woo, > > On 6 March 2014 07:43, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: >> Hello Sachin, >> >> On 2014? 03? 05? 20:42, Sachin Kamat wrote: >>> On 5 March 2014 16:38, Seung-Woo Kim <sw0312.kim@samsung.com> wrote: >> >> (...) >> >>>> - dev->bank1 = dev->bank1; >>> >>> Are you sure this isn't some kind of typo? If not then your commit >>> description is too verbose >>> to actually say that the code is redundant and could be removed. The >>> code here is something like >>> >>> a = a; >>> >>> which does not make sense nor add any value and hence redundant and >>> could be removed. >> >> Right, this meaningless code can be simply removed as like the first >> version. Anyway this redundant made from change of address type in >> earlier patch. So I tried to describe that. > > What Kamil meant was that it is not a good practice to leave the > commit description > blank however trivial the patch might be. So a single line stating the > obvious should > be sufficient in this case. > Ok, I will re-send with just simple statement. Thanks, - Seung-Woo Kim
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c index 2475a3c..ee05f2d 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c @@ -44,8 +44,6 @@ int s5p_mfc_alloc_firmware(struct s5p_mfc_dev *dev) return -ENOMEM; } - dev->bank1 = dev->bank1; - if (HAS_PORTNUM(dev) && IS_TWOPORT(dev)) { bank2_virt = dma_alloc_coherent(dev->mem_dev_r, 1 << MFC_BASE_ALIGN_ORDER, &bank2_dma_addr, GFP_KERNEL);
There was assignment of memory bank with dma address converted from physical address. But allocation has been changed with dma function, so the assignment is not necessary. Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com> --- change from v1 - fixes subject and adds proper description --- drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)