Bug in board/logicpd/omap3som/omap3logic.h ???

Dear Adam,
commit 25e4ff45b17 "ARM: omap3_logic: Enable OMAP EHCI support for SOM-LV Boards" added (among other things) these lines:
... 243 MUX_VAL(CP(MCSPI2_SOMI), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA5*/ 244 MUX_VAL(CP(MCSPI2_CS0), (IEN | PTD | EN | M0)); /*HSUSB2_DATA6*/ 245 MUX_VAL(CP(MCSPI2_CLK), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA7*/ 246 MUX_VAL(CP(SYS_BOOT2), (IEN | PTD | DIS | M4)) /* GPIO_4 */ 247 MUX_VAL(CP(ETK_D10_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_CLK*/ 248 MUX_VAL(CP(ETK_D11_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_STP*/ 249 MUX_VAL(CP(ETK_D12_ES2), (IEN | PTU | DIS | M3)); /*HSUSB2_DIR*/ ...
All the entries in set_muxconf_regs() have a terminating semicolon - all except the entry in line # 246.
I reckon this is a mistake and should be fixed?
Best regards,
Wolfgang Denk

On Mon, Sep 27, 2021 at 04:45:01PM +0200, Wolfgang Denk wrote:
Dear Adam,
commit 25e4ff45b17 "ARM: omap3_logic: Enable OMAP EHCI support for SOM-LV Boards" added (among other things) these lines:
... 243 MUX_VAL(CP(MCSPI2_SOMI), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA5*/ 244 MUX_VAL(CP(MCSPI2_CS0), (IEN | PTD | EN | M0)); /*HSUSB2_DATA6*/ 245 MUX_VAL(CP(MCSPI2_CLK), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA7*/ 246 MUX_VAL(CP(SYS_BOOT2), (IEN | PTD | DIS | M4)) /* GPIO_4 */ 247 MUX_VAL(CP(ETK_D10_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_CLK*/ 248 MUX_VAL(CP(ETK_D11_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_STP*/ 249 MUX_VAL(CP(ETK_D12_ES2), (IEN | PTU | DIS | M3)); /*HSUSB2_DIR*/ ...
All the entries in set_muxconf_regs() have a terminating semicolon - all except the entry in line # 246.
I reckon this is a mistake and should be fixed?
Interesting. MUX_VAL is from arch/arm/include/asm/arch-omap3/mux.h and includes a semicolon, so that's how this didn't trip anything up. Most of the time it's not used with one either, from a quick grep.

On Mon, Sep 27, 2021 at 9:49 AM Tom Rini trini@konsulko.com wrote:
On Mon, Sep 27, 2021 at 04:45:01PM +0200, Wolfgang Denk wrote:
Dear Adam,
commit 25e4ff45b17 "ARM: omap3_logic: Enable OMAP EHCI support for SOM-LV Boards" added (among other things) these lines:
... 243 MUX_VAL(CP(MCSPI2_SOMI), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA5*/ 244 MUX_VAL(CP(MCSPI2_CS0), (IEN | PTD | EN | M0)); /*HSUSB2_DATA6*/ 245 MUX_VAL(CP(MCSPI2_CLK), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA7*/ 246 MUX_VAL(CP(SYS_BOOT2), (IEN | PTD | DIS | M4)) /* GPIO_4 */ 247 MUX_VAL(CP(ETK_D10_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_CLK*/ 248 MUX_VAL(CP(ETK_D11_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_STP*/ 249 MUX_VAL(CP(ETK_D12_ES2), (IEN | PTU | DIS | M3)); /*HSUSB2_DIR*/ ...
All the entries in set_muxconf_regs() have a terminating semicolon - all except the entry in line # 246.
I reckon this is a mistake and should be fixed?
Interesting. MUX_VAL is from arch/arm/include/asm/arch-omap3/mux.h and includes a semicolon, so that's how this didn't trip anything up. Most of the time it's not used with one either, from a quick grep.
I am guessing it was left out by accident. I would not have intentionally done that. I can submit a patch to make it consistent if you want.
adam
-- Tom

Dear Adam,
In message CAHCN7x+_3iZtTjL9i0QL=w-iLC3imegNH38Q=RyT6wz7=Ovicw@mail.gmail.com you wrote:
commit 25e4ff45b17 "ARM: omap3_logic: Enable OMAP EHCI support for SOM-LV Boards" added (among other things) these lines:
... 243 MUX_VAL(CP(MCSPI2_SOMI), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA5*/ 244 MUX_VAL(CP(MCSPI2_CS0), (IEN | PTD | EN | M0)); /*HSUSB2_DATA6*/ 245 MUX_VAL(CP(MCSPI2_CLK), (IEN | PTD | DIS | M0)); /*HSUSB2_DATA7*/ 246 MUX_VAL(CP(SYS_BOOT2), (IEN | PTD | DIS | M4)) /* GPIO_4 */ 247 MUX_VAL(CP(ETK_D10_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_CLK*/ 248 MUX_VAL(CP(ETK_D11_ES2), (IDIS | PTU | DIS | M3)); /*HSUSB2_STP*/ 249 MUX_VAL(CP(ETK_D12_ES2), (IEN | PTU | DIS | M3)); /*HSUSB2_DIR*/ ...
All the entries in set_muxconf_regs() have a terminating semicolon - all except the entry in line # 246.
I reckon this is a mistake and should be fixed?
Interesting. MUX_VAL is from arch/arm/include/asm/arch-omap3/mux.h and includes a semicolon, so that's how this didn't trip anything up. Most of the time it's not used with one either, from a quick grep.
I am guessing it was left out by accident. I would not have intentionally done that. I can submit a patch to make it consistent if you want.
Yes, it should be made xonsistent, please - otherwise everybody seeing this wonders what is going on here.
Tom says there is already a semicolon in the macro and most use cases don;t have one, so the fix should probably be to keep that line and remove the other semicolons ?
Best regards,
Wolfgang Denk
participants (3)
-
Adam Ford
-
Tom Rini
-
Wolfgang Denk