On Tue, Jun 16, 2020 at 11:55:46AM +0000, Shiju Jose wrote: > >From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi- > >owner@xxxxxxxxxxxxxxx] On Behalf Of Andy Shevchenko > >On Tue, Jun 16, 2020 at 09:12:56AM +0000, Shiju Jose wrote: > >> >From: Andy Shevchenko [mailto:andriy.shevchenko@xxxxxxxxxxxxxxx] > >> >On Mon, Jun 15, 2020 at 11:15:52AM +0100, Shiju Jose wrote: ... > >> >> +#define HISI_PCIE_CORE_PORT_ID(v) (((v) % 8) << 1) > >> > > >> >% -> & ? > >> (((v) % 8) << 1) is correct. We can make bit operation instead. > > > >y % x is usually being used when we consume y / x or in cases when y is > >advanced and we need to keep it under some threshold. > > > >Here it's not obvious to me, and usual pattern is to use bitwise operations. > > > >In any case some clarification is needed. > We want (v % 8) * 2 here to get the core port id, a numerical value but not a bit mask. > Maybe you want us to use ((v) & 7) << 1? > please point it out if I understand wrong. I understand the result, I do not understand the properties of v. So, looks like a) (v & 7) << 1 // take 3 LSBs from v and shift right to get port id pair (looks like) b) (v % 8) * 2 // get next free port or circle over 0 if no free pair found Add some comment explaining what's going on. ... > >> >> + switch (id) { > >> >> + case HISI_PCIE_SUB_MODULE_ID_AP: return "AP Layer"; > >> >> + case HISI_PCIE_SUB_MODULE_ID_TL: return "TL Layer"; > >> >> + case HISI_PCIE_SUB_MODULE_ID_MAC: return "MAC Layer"; > >> >> + case HISI_PCIE_SUB_MODULE_ID_DL: return "DL Layer"; > >> >> + case HISI_PCIE_SUB_MODULE_ID_SDI: return "SDI Layer"; > >> >> + } > >> > > >> >match_string() ? > >> > >> match_string() does not work here because we need sub module id -> > >> string conversion. > > > >Why? Are you using non-sequential (a.k.a. sparse) values? > These are the sequential values. > I mean in this case we do not have the third parameter to the match_string(), > string to match with the strings in the array, > we just have the value for the sub module id. > Can you suggest some example of match_string() > for the similar case? Ah, I realize, this is the opposite, but still perhaps better to have like this: static const char * const foo[] = { "AB", "CD", }; const char *bar(int id) { if (id >= ARRAY_SIZE(foo)) return "unknown"; // whatever return foo[id]; } -- With Best Regards, Andy Shevchenko
