Проблема RMW при использовании GPIO

[Legoos]

Доброго дня.

В ядре 3.4 при сбросе внешних радио чипов в процессоре MT7621 используется Read-Modify-Write операция с регистром RALINK_GPIO_DATA0 тут и тут. Суть проблемы в том, что операция выполняется не тат как ожидается, потому что при чтении берутся значения не ранее записанные в этот регистр, а непосредственно с gpio выводов, если gpio в режиме входа (см. руководство по программированию, стр. 67). Для записи без модификации ранее записанных данных правильно использовать регистры DSET и DCLR. Предлагаю следующий патч:

diff --git a/arch/mips/rt2880/pci.c b/arch/mips/rt2880/pci.c
index a2a007cf6..4a6363e24 100644
--- a/arch/mips/rt2880/pci.c
+++ b/arch/mips/rt2880/pci.c
@@ -112,7 +112,8 @@ static int pcie_link_status = 0;
 #define GPIO_PCIE_PORT2			7	// TXD3 (I2S_WS)
 #endif
 #define RALINK_GPIO_CTRL0		*(volatile u32 *)(RALINK_PIO_BASE + 0x00)
-#define RALINK_GPIO_DATA0		*(volatile u32 *)(RALINK_PIO_BASE + 0x20)
+#define RALINK_GPIO_DSET0		*(volatile u32 *)(RALINK_PIO_BASE + 0x30)
+#define RALINK_GPIO_DCLR0		*(volatile u32 *)(RALINK_PIO_BASE + 0x40)
 #endif
 
 #define ASSERT_SYSRST_PCIE(val)		do { \
@@ -639,7 +640,7 @@ int __init init_ralink_pci(void)
 	mdelay(50);
 	RALINK_GPIO_CTRL0 |= val;			// switch PERST_N pin to output mode
 	mdelay(50);
-	RALINK_GPIO_DATA0 &= ~(val);			// fall PERST_N pin (reset peripherals)
+	RALINK_GPIO_DCLR0 = val;			// fall PERST_N pin (reset peripherals)
 #else /* !defined (GPIO_PERST) */
 	RALINK_GPIOMODE &= ~(0x3<<PCIE_SHARE_PIN_SW);	// fall PERST_N pin (reset peripherals)
 #endif
@@ -687,7 +688,7 @@ int __init init_ralink_pci(void)
 #if defined (CONFIG_PCIE_PORT2)
 	val |= (0x1<<GPIO_PCIE_PORT2);
 #endif
-	RALINK_GPIO_DATA0 |= val;			// rise PERST_N pin (complete reset peripherals)
+	RALINK_GPIO_DSET0 = val;			// rise PERST_N pin (complete reset peripherals)
 #else /* !defined (GPIO_PERST) */
 	RALINK_PCI_PCICFG_ADDR &= ~(1<<1);		// release PCIRST
 #endif 

 

[sergeyk]

42 минуты назад, Legoos сказал:

Суть проблемы в том, что операция выполняется не тат как ожидается, потому что при чтении берутся значения не ранее записанные в этот регистр, а непосредственно с gpio выводов, если gpio в режиме входа (см. руководство по программированию, стр. 67).

Согласитесь, что

RALINK_GPIO_DCLR0 = val;
...
RALINK_GPIO_DSET0 = val;

тоже не имеют смысла (как и вся процедура сброса), если GPIO находится в режиме входа.

[Legoos]

Процессор GPIO19 сбрасывает pcie периферию, так что 19 настроен на выход. Понятно, что потом при работе с другими выходными GPIO они переинициализируются и скорее всего никто не заметит, что там был мусор. Но правильно использовать DSET и DCLR тем более в драйвере ralink_gpio так и сделано.

[Legoos]

Если я правильно Вас понял и для сброса используется регистр RSTCTRL, тогда тот код с GPIO вообще не нужен.

[sergeyk]

48 минут назад, Legoos сказал:

Если я правильно Вас понял и для сброса используется регистр RSTCTRL, тогда тот код с GPIO вообще не нужен.

Нет, вы меня неправильно поняли.

В моём документе про регистры RALINK_GPIO_DATAX ясно написано:

Цитата

These data registers store current GPIO data value for GPIO input mode, or output driven value for GPIO output mode.

Регистры RALINK_GPIO_DATAX доступны для чтения и записи.

[Le ecureuil]

А какую проблему пытается решить этот патч?

[Legoos]

1 час назад, Le ecureuil сказал:

А какую проблему пытается решить этот патч?

Тут на примере другого микроконтроллера, но суть та же. Ожидаем, что прочитаем ранее записанное значение, наложим маску и тем самым изменим нужный нам бит. А по факту вычитаем значение с выводов контроллера (если какие-то gpio настроены на вход) получим мусор и запишем обратно. Нужный бит установится и ещё куча случайных, потом где-то в коде на выход развернётся другой бит, а там уже 1 или 0. Тогда нужно предварительно записывать нужное значение, а потом разворачивать gpio на выход. Но это всё не нужно, потому что есть специально предназначенные для этого регистры.

Если настройка выводов не изменяется после инициализации в u-boot, то можно и так оставить, просто потенциально где-то потом может стрельнуть.

2 часа назад, sergeyk сказал:

Нет, вы меня неправильно поняли.

В моём документе про регистры RALINK_GPIO_DATAX ясно написано:

Регистры RALINK_GPIO_DATAX доступны для чтения и записи.

Вот именно, что явно написано: при чтении текущие значения GPIO(если на вход), при записи предыдущие значения(если на выход). Читается весь регистр целиком, кто отслеживает какие биты на вход, а какие на выход - никто, потому что есть спец регистры, чтобы с этим не заморачиваться.

[Legoos]

Этот код перекочевал с какого-то предыдущего процессора, где по другому было нельзя. Вот в драйвере видно, что RMW операцию делают над регистром данных для установки или сброса бита на каком-то другом процессоре. И далее по коду для MT7621 уже для этого используют другие регистры, а регистр данных только читают.

[sergeyk]

4 часа назад, Legoos сказал:

Читается весь регистр целиком, кто отслеживает какие биты на вход, а какие на выход - никто, потому что есть спец регистры, чтобы с этим не заморачиваться.

Вы так и не объяснили, в чём проблема.
Технически работают оба варианта кода. Единственное отличие в том, что DSET и DCLR изменяют заданные биты атомарно, а DATA требует предварительного чтения, что может быть проблемой, если нет внешней синхронизации доступа к GPIO, но init_ralink_pci вызывается, когда в системе работает один процессор, так что в этом коде никакой разницы нет.

[Legoos]

46 минут назад, sergeyk сказал:

Вы так и не объяснили, в чём проблема.

Ну я уж не знаю как ещё можно объяснить, пусть останется капризом перфекциониста. Про то что значение в регистре может изменится в прерывании или другим потоком я даже и не подумал. Ну вообще шевелить пином, когда есть специальные биты в регистре RSTCTRL как то странно. Наверняка была причина, как я понял это всё из openwrt произрастает. Пользуясь случаем благодарю за отличную операционку и железо. Отдельное спасибо Илье за zram, r8152 и exfat.

[Le ecureuil]

Это не в openwrt вопрос, а в авторах процессора. Если поискать ralink apsoc sdk и посмотреть этот код, то сразу станет видно, что это авторы так написали (хотя у них порой мысли слишком брызжут через край :D). Ну и учитывая, что ядро 3.4 и ПО 2.16 уже legacy, то я ничего исправлять чисто ради красоты не хотел бы.

[Legoos]

В 05.07.2020 в 20:36, Legoos сказал:

Ну вообще шевелить пином, когда есть специальные биты в регистре RSTCTRL как то странно.

Похоже на небольшой просчёт в дизайне SOC. Поскольку G14(когда он PERST_N) является выходом с открытым стоком с внутренней подтяжкой с током 4 мА, которого видимо не достаточно, чтобы вытянуть единицу, когда на порту висят больше одного чипа. Соответственно нужно тянуть резистором, но этот же вывод является бутстрапом для OCP_RATIO и для делителя 1:3 нужно тянуть в ноль. Чтобы не ставить подтяжку к питанию этот пин используется как GPIO, софтовый workaround аппаратного косячка. 

[Le ecureuil]

Вполне возможно, errata в аппаратной части явление довольное частое, и в каждом процессоре есть что-то, что приходилось обходить разными ухищрениями.

Другое дело, когда это как в ситуации с ppe в 7620 приводило к его зависанию на UDP, и это очень печально; другое дело когда нужно пару лишних раз записать в регистр для устранения "дребезга" .