Jump to content
Legoos

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

Recommended Posts

Доброго дня.

В ядре 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 

 

Share this post


Link to post
Share on other sites
42 минуты назад, Legoos сказал:

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

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

RALINK_GPIO_DCLR0 = val;
...
RALINK_GPIO_DSET0 = val;

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
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 доступны для чтения и записи.

Share this post


Link to post
Share on other sites
1 час назад, Le ecureuil сказал:

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

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

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

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

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

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

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

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

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites
4 часа назад, Legoos сказал:

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

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

  • Thanks 1

Share this post


Link to post
Share on other sites
46 минут назад, sergeyk сказал:

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

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

Share this post


Link to post
Share on other sites

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

  • Thanks 1

Share this post


Link to post
Share on other sites
В 05.07.2020 в 20:36, Legoos сказал:

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

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

Share this post


Link to post
Share on other sites

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

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

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


  • Recently Browsing   0 members

    No registered users viewing this page.

×
×
  • Create New...