Hi Uwe, On Thu, Dec 10, 2020 at 9:05 PM Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > > @@ -111,6 +118,8 @@ > > > > > > #define SN_LINK_TRAINING_TRIES 10 > > > > > > +#define SN_PWM_GPIO 3 > > > > So this maps to the GPIO4 described in sn65dsi86 datasheet. I'm > > wondering if it's more readable to define the following SHIFT constants > > (your code), and use GPIO_MUX_GPIO4_SHIFT >> 2 where you need GPIO > > offset? > > > > #define GPIO_MUX_GPIO1_SHIFT 0 > > #define GPIO_MUX_GPIO2_SHIFT 2 > > #define GPIO_MUX_GPIO3_SHIFT 4 > > #define GPIO_MUX_GPIO4_SHIFT 6 > > > > If you agree, you may consider to integrate this patch beforehand: > > > > https://github.com/shawnguo2/linux/commit/7cde887ffb3b27a36e77a08bee3666d14968b586 > > My preferred way here would be to add a prefix for the other constants. > It (IMHO) looks nicer and > > GPIO_INPUT_SHIFT > > looks like a quite generic name for a hardware specific definition. While this looks like a reasonable argument, I also like the naming choice for these constants in the beginning for that distinction between registers and bits. And changing the names the other way around means there will be a much bigger diffstat, which I would like to avoid. I suggest let's just focus on what really matters here - keep the naming consistent, so that people do not get confused when they want to add more constants in there. Shawn > (Even if up to now there is no other code location using this name.)
