Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions drivers/pinctrl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

zephyr_library()
zephyr_library_sources(common.c)
zephyr_library_sources_ifdef(CONFIG_PINCTRL_NXP_MCUX pinctrl_nxp_mcux.c)
7 changes: 7 additions & 0 deletions drivers/pinctrl/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,11 @@ config PINCTRL_DYNAMIC
runtime. This can be useful, for example, to change the pins assigned to a
peripheral at early boot stages depending on a certain input.

config PINCTRL_NXP_MCUX
bool "Pin controller driver for NXP Kinetis MCUs"
depends on SOC_FAMILY_KINETIS
default y
help
Enable pin controller driver for NXP Kinetis MCUs

endif # PINCTRL
17 changes: 17 additions & 0 deletions drivers/pinctrl/pinctrl_nxp_mcux.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright (c) 2021 Linaro Limited.
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <drivers/pinctrl.h>

int pinctrl_configure_pins(const pinctrl_soc_pin_t *pins, uint8_t pin_cnt,
uintptr_t reg)
{
for (uint8_t i = 0U; i < pin_cnt; i++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (uint8_t i = 0U; i < pin_cnt; i++) {
uint8_t i;
for (i = 0U; i < pin_cnt; i++) {

pins[i].port_reg->PCR[pins[i].pin] = pins[i].mux;
Copy link

@jgediya jgediya Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some more valid bits in the PCR register, shouldn't this be read, clear lowest 16 bits, and '|' mux value operation? Maybe in some scenario, this can create problems?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which bits would you preserve here? The idea is that all these bits are controlled by the pinctrl driver based on the devicetree properties.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IRQC bits, which are currently configured by gpio_mcux.c driver for interrupt configuration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Right.

}

return 0;
}
5 changes: 5 additions & 0 deletions dts/bindings/pinctrl/nxp,kinetis-pinmux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ compatible: "nxp,kinetis-pinmux"
include:
- name: base.yaml
- name: pincfg-node.yaml
child-binding:
property-allowlist:
- bias-pull-down
- bias-pull-up
- drive-open-drain

properties:
reg:
Expand Down
3 changes: 3 additions & 0 deletions soc/arm/nxp_kinetis/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ zephyr_sources_ifdef(CONFIG_KINETIS_FLASH_CONFIG flash_configuration.c)

add_subdirectory(${SOC_SERIES})

# This is for access to pinmux macros
zephyr_include_directories(common)

zephyr_linker_sources_ifdef(CONFIG_KINETIS_FLASH_CONFIG
ROM_START
SORT_KEY ${CONFIG_KINETIS_FLASH_CONFIG_OFFSET}
Expand Down
1 change: 1 addition & 0 deletions soc/arm/nxp_kinetis/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
config SOC_FAMILY_KINETIS
bool
select HAS_SEGGER_RTT
select PINCTRL

if SOC_FAMILY_KINETIS

Expand Down
76 changes: 76 additions & 0 deletions soc/arm/nxp_kinetis/common/pinctrl_soc.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (c) 2021 Linaro Limited.
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @file
* Public APIs for pin control drivers
*/
Comment on lines +6 to +9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed


#ifndef ZEPHYR_SOC_ARM_NXP_KINETIS_COMMON_PINCTRL_SOC_H_
#define ZEPHYR_SOC_ARM_NXP_KINETIS_COMMON_PINCTRL_SOC_H_

/**
* @brief Pin Controller Interface (NXP Kinetis)
* @defgroup pinctrl_interface_nxp_kinetis Pin Controller Interface
* @ingroup pinctrl_interface
* @{
*/
Comment on lines +14 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this file doesn't expose any public API this can likely be removed


#include <devicetree.h>
#include <zephyr/types.h>

#ifdef __cplusplus
extern "C" {
#endif

/** @cond INTERNAL_HIDDEN */

/** Type for NXP Kinetis pin. */
typedef struct pinctrl_soc_pin {
PORT_Type *port_reg;
uint8_t pin;
uint16_t mux;
} pinctrl_soc_pin_t;

#define PINCTRL_SOC_PINS_ELEM_INIT(node_id) \
{ \
.port_reg = (PORT_Type *)DT_REG_ADDR(DT_PARENT(node_id)), \
.pin = DT_PROP_BY_IDX(node_id, nxp_kinetis_port_pins, 0), \
.mux = PORT_PCR_MUX(DT_PROP_BY_IDX(node_id, nxp_kinetis_port_pins, 1)) | \
(DT_PROP(node_id, bias_pull_up) & (PORT_PCR_PE_MASK | PORT_PCR_PS_MASK)) | \
(DT_PROP(node_id, bias_pull_down) & PORT_PCR_PS_MASK) | \
Copy link

@jgediya jgediya Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't PORT_PCR_PE_MASK be used for pull-down?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This should be (DT_PROP(node_id, bias_pull_down) & PORT_PCR_PE_MASK) |.

(DT_PROP(node_id, drive_open_drain) & PORT_PCR_ODE_MASK), \
Copy link

@jgediya jgediya Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PORT_PCR_ODE_MASK is 0x20, If DT_PROP(node_id, drive_open_drain) return true(1) then '&' of them (1 & 0x20) will be 0. Instead we want 5th bit to be set here. Isn't it? Should we write here (DT_PROP(node_id, drive_open_drain) << PORT_PCR_ODE_SHIFT)? If this is true, then same can be applied to other '&' operations as well.

},

/**
* @brief Utility macro to initialize each pin.
*
* @param node_id Node identifier.
* @param state_prop State property name.
* @param idx State property entry index.
*/
#define Z_PINCTRL_STATE_PIN_INIT(node_id, state_prop, idx) \
PINCTRL_SOC_PINS_ELEM_INIT(DT_PROP_BY_IDX(node_id, state_prop, idx))

/**
* @brief Utility macro to initialize state pins contained in a given property.
*
* @param node_id Node identifier.
* @param prop Property name describing state pins.
*/
#define Z_PINCTRL_STATE_PINS_INIT(node_id, prop) \
{DT_FOREACH_PROP_ELEM(node_id, prop, Z_PINCTRL_STATE_PIN_INIT)}

/** @endcond */

#ifdef __cplusplus
}
#endif

/**
* @}
*/

#endif /* ZEPHYR_SOC_ARM_NXP_KINETIS_COMMON_PINCTRL_SOC_H_ */