Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codegen for an C structure that stores pinmux definitions #23138

Closed
nandojve opened this issue Feb 27, 2020 · 1 comment
Closed

Codegen for an C structure that stores pinmux definitions #23138

nandojve opened this issue Feb 27, 2020 · 1 comment
Labels

Comments

@nandojve
Copy link
Member

Is your feature request related to a problem? Please describe.

This proposes a small integration to allow codegen create a C structure to store small data about pins. The end result can be used by gpio/pinmux/pinctrl drivers.
The discussion about the API can be followed at #22748. The intention here is not solve all the problems but give perspective and maybe give a option to store the data.

Describe the solution you'd like

This will try to solve the storage information for the SAMR21 device. It have a pinmux implementation but there isn't pinctrl definitions.

  • Device Tree Example
/ {
	...
	pioA: pinctrl@0 {
		compatible = "atmel,samd21-pinctrl";
		reg = <0x0 0x200>;

		usart0_default {
			pinmux = <&pinmux_a pin_number_TX function>,
				 <&pinmux_b pin_number_RX function>;
			bias-disable;
		};
	};
};

&sercom0 {
	status = "okay";
	compatible = "atmel,sam0-uart";
	current-speed = <115200>;
	rxpo = <1>;
	txpo = <0>;
	
	pinctrl-0 = &usart0_default;
};

At run time system need access GPIO, Pinmux and Pinctrl DT definitions. To help with that, a C structure can handle most of definitions. This proposal consists in describe a way to simple store on flash the some pins definitions.

  • Add <platform>-pinfunc.h file for any platform

This file include/dt-bindings/pinctrl/<platform>-pinfunc.h will store macros and definitions for codegen. A good example is stm32_pinmux.h

It will be necessary create a C structure, define pin muxing names, a prolog and an epilog.

  • C structure
struct atmel_samd21_pinmux_struct {
	u8_t	pinmux_id,
	u8_t	pin_number,
	u8_t	function_id,
	u8_t	pin_flags,
};
  • Pin Mux function definitions
#define	Fgpio		0
#define	FA		1
#define	FB		2
#define	FC		3
#define	FD		4
  • prolog

Will be called by codegen to create basic header for the C structure instance. As an simple example it could be:

#define PROLOG "static atmel_samd21_pinmux_struct atmel_samd21_pinmux_inst[] = {\n"

  • Macro definition

Define a macro that will be called at codegen and will be responsible to fill up the C structure.

#define PINMUX_EMIT "\t{ %d, %d, %d, %d},\n"

  • epilog

Will be called by codegen to finish the C structure instance.

#define EPILOG "}\n"

  • Concept proof

The codegen will process normally all device tree nodes. The last step can create a new file to store the struct instance. The codegen will go through all DT nodes searching for "pinctrl-x" property. For every match, it will get the property value and store on a list to avoid duplicate and process other possible checks. For the current example, the node is "usart0_default". Codegen need then evaluate the node and emit all entries on pinmux property plus a number that will represent all GPIO flags.

For the example:

/* The snip below */
usart0_default {
	pinmux = <&pinmux_a 1 FA>,
		 <&pinmux_e 2 Fgpio>;
		bias-disable;
	};
&sercom0 {
	pinctrl-0 = &usart0_default;
};
/* became this */
static struct atmel_samd21_pinmux_struct atmel_samd21_pinmux_inst[] = {
	/* sercom0/pinctrl-0/usart0_default */
	{0, 1, FA, 0x80},	/* pinmux_a index = 0, pin = 1, function = FA, bias-disabled = 0x80 */
	{4, 2, Fgpio, 0x80},	/* pinmux_e index = 4, pin = 2, function = Fgpio, bias-disabled = 0x80 */
}
  • New Pinmux Driver

Before

static int board_pinmux_init(struct device *dev)
{
	struct device *muxa = device_get_binding(DT_ATMEL_SAM0_PINMUX_PINMUX_A_LABEL);
	struct device *muxb = device_get_binding(DT_ATMEL_SAM0_PINMUX_PINMUX_B_LABEL);

	ARG_UNUSED(dev);

#if DT_ATMEL_SAM0_UART_SERCOM_0_BASE_ADDRESS
	/* SERCOM0 on RX=PA11, TX=PA10 */
	pinmux_pin_set(muxa, 11, PINMUX_FUNC_C);
	pinmux_pin_set(muxa, 10, PINMUX_FUNC_C);
#endif

	...
	return 0;
}

Became

#include <init.h>
#include <drivers/pinmux.h>

static int board_pinmux_init(struct device *dev)
{
	struct device *pinmux_ctrl[DT_ATMEL_SAM0_PINMUX_IDX_END - DT_ATMEL_SAM0_PINMUX_IDX_START];
	struct atmel_samd21_pinmux_struct *pin_inst;
	u32_t idx;

	ARG_UNUSED(dev);

	for(idx = DT_ATMEL_SAM0_PINMUX_IDX_START; idx < DT_ATMEL_SAM0_PINMUX_IDX_END; idx++)
	{
		pinmux_ctrl[idx] = device_get_binding_by_index(idx);
	}
	
	for(idx = 0; idx < ARRAY_SIZE(atmel_samd21_pinmux_inst); idx++)
	{
		pin_inst = atmel_samd21_pinmux_inst[idx];
		pinmux_pin_set(pinmux_ctrl[pin_inst->pinmux_id], pin_inst->pin_number, pin_inst->function_id);
	}
	
	return 0;
}

SYS_INIT(board_pinmux_init, PRE_KERNEL_1, CONFIG_PINMUX_INIT_PRIORITY);

Additional context
I think this structure can store the data to be used for gpio/pinmux/pinctrl in the smallest possible value. The GPIO/pinmux driver can simple run over that structure and initialize the pins. To accomplish that will be need some additional API.

  • device_get_binding_by_index return a device driver by the index of storage. It is very fast because is just a dereference pointer. The codegen need emit 3 new informations:

  • 1: For any "LABEL" that exists an equivalent INDEX exists. The user can used both and may replace the label since the index is a valid info and can be used.

  • 2/3: Any group need the START and END definition to allow loop instructions. Since we don't store "compat" string we can't dynamically discover the indexes.

Benefits

  • We start to store an important and indispensable information.
  • We can define the standard as Linux and use the Linux DT too when appropriated.
  • User and SoC developers can start to populate DT with all Pinmux/Pinctrl information. I like Linux idea that say "even we are not using right now we need add the information for future use". This will enable things automatically in future.
  • Drop all GPIO definitions on dts_fixup files
  • Give free option to user define and use how many pins the board really need. Ex. If we define dts_fixup for 4 pin usart but user need only 2 we will initialize that anyway. With this user can define their own pinmux definition.
@nandojve nandojve added the Feature Request A request for a new feature label Feb 27, 2020
@nandojve
Copy link
Member Author

nandojve commented Feb 27, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants