On Thu, Dec 10, 2020 at 10:40:36PM +0800, Shawn Guo wrote: > 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. In my eyes the bigger diffstat is justified. As I wrote, GPIO_INPUT_SHIFT isn't used in other files, but please look how many definitions there are for RESET. The usefulness of ctags/cscope is quite reduced if generic terms are used this way. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
