Quoting David Dai (2019-01-16 16:54:39) > > On 1/14/2019 8:47 AM, Stephen Boyd wrote: > > Quoting David Dai (2019-01-11 16:56:14) > >> On 1/9/2019 11:28 AM, Stephen Boyd wrote: > >>> Quoting David Dai (2018-12-13 18:35:04) > >>>> + > >>>> +#define BCM_TCS_CMD(valid, vote) \ > >>>> + (BCM_TCS_CMD_COMMIT_MASK | \ > >>>> + ((valid) << BCM_TCS_CMD_VALID_SHIFT) | \ > >>>> + ((cpu_to_le32(vote) & \ > >>> Why? > >> Sorry, could you clarify this question? If you're referring to the > >> cpu_to_le32, shouldn't we be explicit about converting endianness even > >> if it might be redundant for this particular qcom platform? > > Is only the vote part of the message in little endian format and the > > rest is native CPU endianess? It's very odd to see that jammed in the > > middle of a bit packing statement like that. Typically the whole u32 > > would be in one or the other endianness. Also, sparse right complains > > about this macro and it's usage, so something is wrong. > Point taken, I'll leave it out of the macro for now. > > I think one other problem is that rpmh API doesn't really talk about > > endianness here and that's busted. So if you want to fix endianness > > issues that needs to be fixed first. > > Since a similar macro is being used as part of the interconnect provider > driver, I was thinking a better place for this macro might be in the > tcs.h as part of the rpmh driver? I could submit a different patch for > rpmh that mentions endianness along with this change. Sure that's fine. But be warned that making a dependency across kernel trees is best avoided. I would do that sort of cleanup and consolidation after the two drivers are merged upstream so that it can go via either tree.
