type inconsistency in drivers/tty/serial/msm_serial_hs.c

type inconsistency in drivers/tty/serial/msm_serial_hs.c

[Ilia Mirkin]

Hello,

I've been playing around with spatch, and noticed the following type
inconsistency. I thought about just fixing it myself, but since this
involves DMA's to the device, I decided it was best to leave it up to
someone who had a device or at least understood how the whole thing
was supposed to function.

In uartdm_init_port, around line 1540:

tx->command_ptr_ptr = kmalloc(sizeof(u32 *), GFP_KERNEL | __GFP_DMA);

But command_ptr_ptr is a u32*, which means that the kzalloc should be
for sizeof(u32). This has further implications for the dma_map_single
a few lines down (and corresponding dma_unmap_single's).

Or perhaps it should go the other way, and command_ptr_ptr should be a
u32** (as its name might suggest), with no other changes being
required. Although this seems unlikely given the usage around line
812:

*tx->command_ptr_ptr = CMD_PTR_LP | DMOV_CMD_ADDR(tx->mapped_cmd_ptr);

where CMD_PTR_LP is 1<<31. I guess none of this is a particularly big
deal since sizeof(u32) == sizeof(u32*) == sizeof(u32**) on a 32-bit
platform... but it's nice to be consistent.

  -ilia

P.S. Are you guys missing an entry in MAINTAINERS for this file? I
just noticed that you supported msm_serial.c, figured this was
related. If not, please repoint me in the right direction.

Re: type inconsistency in drivers/tty/serial/msm_serial_hs.c

[David Brown]

On Fri, Nov 25, 2011 at 02:58:10PM -0500, Ilia Mirkin wrote:

> P.S. Are you guys missing an entry in MAINTAINERS for this file? I
> just noticed that you supported msm_serial.c, figured this was
> related. If not, please repoint me in the right direction.

I'll include Mayank Rana, who contributed the initial commit.  This
patch appears to have gone through Greg KH.

I'll also take a look at this, since it does apply to the MSM
hardware.

Thanks,
David

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

Re: type inconsistency in drivers/tty/serial/msm_serial_hs.c

[Mayank Rana]

Hi Ilia,
On 11/26/2011 1:28 AM, Ilia Mirkin wrote:
> Hello,
>
> I've been playing around with spatch, and noticed the following type
> inconsistency. I thought about just fixing it myself, but since this
> involves DMA's to the device, I decided it was best to leave it up to
> someone who had a device or at least understood how the whole thing
> was supposed to function.
>
> In uartdm_init_port, around line 1540:
>
> tx->command_ptr_ptr = kmalloc(sizeof(u32 *), GFP_KERNEL | __GFP_DMA);
>
> But command_ptr_ptr is a u32*, which means that the kzalloc should be
> for sizeof(u32). This has further implications for the dma_map_single
> a few lines down (and corresponding dma_unmap_single's).
>
> Or perhaps it should go the other way, and command_ptr_ptr should be a
> u32** (as its name might suggest), with no other changes being
> required. Although this seems unlikely given the usage around line
> 812:
>
> *tx->command_ptr_ptr = CMD_PTR_LP | DMOV_CMD_ADDR(tx->mapped_cmd_ptr);
>
> where CMD_PTR_LP is 1<<31. I guess none of this is a particularly big
> deal since sizeof(u32) == sizeof(u32*) == sizeof(u32**) on a 32-bit
> platform... but it's nice to be consistent.
Yes. It seems that there is type inconsistency in both Tx and Rx Command
ptr_ptr.

The first approach which you have mentioned seems to be way to go with 
it. I will be sending the fix for the same, once able to verify the change.

Thanks for pointing out this.
>
>    -ilia
>
> P.S. Are you guys missing an entry in MAINTAINERS for this file? I
> just noticed that you supported msm_serial.c, figured this was
> related. If not, please repoint me in the right direction.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Regards,
_-_Mayank Rana_-_
-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.