Re: [PATCH/RFC] staging: dwc: Moving all hwcfg accesses to one place

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Paul,

> Overall this seems to be an improvement, so I have no objections. There
> is a pretty high likelihood of something getting broken in the translation,
> however.
> 
> For example, this:
> +	hw->host_nperio_tx_fifo_size = (readl(hsotg->regs + GNPTXFSIZ) & GNPTXFSIZ_NP_TXF_DEP_MASK) << GNPTXFSIZ_NP_TXF_DEP_SHIFT;
> looks like a bug (shifted the wrong direction).
Yeah, I'll make sure to doublecheck the patch when it is finished. I
already found the above one after I sent over the patch, but since I was
just looking to collect feedback on the basic idea behind the patch, I
hadn't thorougly checked it before sending it.

> You probably should use u32 instead of int for the members of struct
> dwc2_hw_params, otherwise you might introduce a sign bug if any of the
> values get shifted into the high bit and become unexpectedly negative. And
> I would use either u8, bool, or bitfields for the true/false values, to
> save a little space.
I don't think any of the values will exceed 2^31, but using more
appropriate types makes sense. I'll see about using properly sized
bitfields to minimize the memory overhead.

Gr.

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux