You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "lordrasmus (via GitHub)" <gi...@apache.org> on 2025/02/13 14:42:28 UTC
[PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
lordrasmus opened a new pull request, #15837:
URL: https://github.com/apache/nuttx/pull/15837
definitions for the max32660 moved to max32660_gpio.h
the max32690 has much more options for the gpios
so this is a complete new driver
## Summary
adds support for the max32690 gpios
## Impact
should have no impact
## Testing
driver works on my dev board
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#discussion_r1954699687
##########
arch/arm/src/max326xx/max32690/max32690_gpio.c:
##########
@@ -0,0 +1,448 @@
+/****************************************************************************
+ * arch/arm/src/max326xx/max32690/max32690_gpio.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/spinlock.h>
+
+#include "arm_internal.h"
+#include "max326_gpio.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+spinlock_t g_max32690_gpio_lock = SP_UNLOCKED;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_default
+ *
+ * Description:
+ * Configure a pin as a default GPIO input as it was on reset
+ *
+ ****************************************************************************/
+
+static void max32690_default(max32690_gpio_regs_t *gpio,
+ max32690_pinconfig_t *conf)
+{
+ uint32_t pinmask = 1 << conf->pin;
+
+ /* Disable interrupts and wake-up events */
+
+ gpio->inten_clr = pinmask; /* Pin interrupt disabled triggered */
+ gpio->intmode &= ~pinmask; /* Level triggered */
+ gpio->intpol &= ~pinmask; /* Input low triggers */
+ gpio->dualedge &= ~pinmask; /* Disable dual edge */
+ gpio->wken_clr = pinmask; /* Disable wakeup */
+
+ /* Make the pins an input an clear output value */
+
+ gpio->outen_clr = pinmask; /* Disable output drivers */
+ gpio->out_clr = pinmask; /* Set the output value to zero */
+
+ /* Set alternate functions to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ /* enable input connection to pin */
+
+ gpio->inen |= pinmask;
+
+ /* Reset drive strength */
+
+ gpio->ds0 &= ~pinmask;
+ gpio->ds1 &= ~pinmask;
+
+ /* Disable pull up and pull down */
+
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+
+ /* Set VVoltage to V_DDIOH */
+
+ gpio->vssel |= pinmask;
+
+ /* Disable slew and hysteresis */
+
+ gpio->hysen &= ~pinmask;
+ gpio->srsel &= ~pinmask;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_get_gpio_bank_regptr
+ *
+ * Description:
+ * Get the GPIO access struct address based in the GPIO Bank
+ *
+ ****************************************************************************/
+
+max32690_gpio_regs_t *max32690_get_gpio_bank_regptr(int bank)
+{
+ max32690_gpio_regs_t *gpio_regs = 0;
+
+ switch (bank)
+ {
+ case 0: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO0_BASE;
+ break;
+ case 1: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO1_BASE;
+ break;
+ case 2: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO2_BASE;
+ break;
+ case 3: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO3_BASE;
+ break;
+ }
+
+ DEBUGASSERT(gpio_regs != 0);
+
+ return gpio_regs;
+}
+
+static
+void set_pin_electrical_characteristics(max32690_gpio_regs_t *gpio,
+ uint16_t conf,
+ uint32_t pinmask)
+{
+ /* Enable slew and hysteresis */
+
+ if ((conf & GPIO_SLEW) != 0)
+ {
+ gpio->srsel |= pinmask;
+ }
+
+ if ((conf & GPIO_HYSTERESIS) != 0)
+ {
+ gpio->hysen |= pinmask;
+ }
+
+ switch (conf & GPIO_DRIVE_MASK)
+ {
+ case GPIO_DRIVE_0_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_0_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+
+ switch (conf & GPIO_IN_MODE_MASK)
+ {
+ case GPIO_FLOAT:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_WEAK:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_STRONG:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_WEAK:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_STRONG:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel |= ~pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+}
+
+/****************************************************************************
+ * Name: max32690_gpio_config
+ *
+ * Description:
+ * Configure a GPIO pin. Config based on bit-encoded descriptions.
+ *
+ ****************************************************************************/
+
+int max32690_gpio_config(max32690_pinconfig_t pinconf)
+{
+ irqstate_t flags;
+ uint32_t pinmask;
+
+ DEBUGASSERT(pinconf.pin <= GPIO_PINMAX);
+ pinmask = 1 << pinconf.pin;
+
+ /* Modification of all registers must be atomic */
+
+ flags = spin_lock_irqsave(&g_max32690_gpio_lock);
+
+ max32690_gpio_regs_t *gpio;
+ gpio = max32690_get_gpio_bank_regptr(pinconf.gpio_bank);
+
+ /* First, force the pin configuration to the default generic
+ * input state.
+ * So that we know we are starting from a known state.
+ */
+
+ max32690_default(gpio, &pinconf);
+
+ /* Then perform the actual pin configuration. We need only
+ * to set values that are not in the default, reset state.
+ */
+
+ /* Handle the pin function */
+
+ switch (pinconf.config & GPIO_FUNC_MASK)
+ {
+ case GPIO_IO:
+
+ /* first change to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ set_pin_electrical_characteristics(gpio, pinconf.config,
+ pinmask);
+
+ if ((pinconf.config & GPIO_OUTPUT_MODE) == GPIO_OUTPUT_MODE)
+ {
+ /* enable pin output mode */
+
+ gpio->outen_set = pinmask;
+ }
+
+ if ((pinconf.config & GPIO_VALUE_ONE) == GPIO_VALUE_ONE)
+ {
+ gpio->out_set = pinmask; /* Set output high */
+ }
+
+ break;
+
+ case GPIO_ALT1:
+
+ /* first change to I/O (transition to AF1) */
+
+ gpio->en0_set = pinmask;
+
Review Comment:
remove extra line
##########
arch/arm/src/max326xx/max32690/max32690_gpio.c:
##########
@@ -0,0 +1,448 @@
+/****************************************************************************
+ * arch/arm/src/max326xx/max32690/max32690_gpio.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/spinlock.h>
+
+#include "arm_internal.h"
+#include "max326_gpio.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+spinlock_t g_max32690_gpio_lock = SP_UNLOCKED;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_default
+ *
+ * Description:
+ * Configure a pin as a default GPIO input as it was on reset
+ *
+ ****************************************************************************/
+
+static void max32690_default(max32690_gpio_regs_t *gpio,
+ max32690_pinconfig_t *conf)
+{
+ uint32_t pinmask = 1 << conf->pin;
+
+ /* Disable interrupts and wake-up events */
+
+ gpio->inten_clr = pinmask; /* Pin interrupt disabled triggered */
+ gpio->intmode &= ~pinmask; /* Level triggered */
+ gpio->intpol &= ~pinmask; /* Input low triggers */
+ gpio->dualedge &= ~pinmask; /* Disable dual edge */
+ gpio->wken_clr = pinmask; /* Disable wakeup */
+
+ /* Make the pins an input an clear output value */
+
+ gpio->outen_clr = pinmask; /* Disable output drivers */
+ gpio->out_clr = pinmask; /* Set the output value to zero */
+
+ /* Set alternate functions to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ /* enable input connection to pin */
+
+ gpio->inen |= pinmask;
+
+ /* Reset drive strength */
+
+ gpio->ds0 &= ~pinmask;
+ gpio->ds1 &= ~pinmask;
+
+ /* Disable pull up and pull down */
+
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+
+ /* Set VVoltage to V_DDIOH */
+
+ gpio->vssel |= pinmask;
+
+ /* Disable slew and hysteresis */
+
+ gpio->hysen &= ~pinmask;
+ gpio->srsel &= ~pinmask;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_get_gpio_bank_regptr
+ *
+ * Description:
+ * Get the GPIO access struct address based in the GPIO Bank
+ *
+ ****************************************************************************/
+
+max32690_gpio_regs_t *max32690_get_gpio_bank_regptr(int bank)
+{
+ max32690_gpio_regs_t *gpio_regs = 0;
+
+ switch (bank)
+ {
+ case 0: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO0_BASE;
+ break;
+ case 1: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO1_BASE;
+ break;
+ case 2: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO2_BASE;
+ break;
+ case 3: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO3_BASE;
+ break;
Review Comment:
I don't know why nxstyle script didn't get it, but "case X:" should be the only thing in a line.
Refer to max32660/max32660_lowputc.c line 334
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "lordrasmus (via GitHub)" <gi...@apache.org>.
lordrasmus commented on PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#issuecomment-2660886094
Is fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "lordrasmus (via GitHub)" <gi...@apache.org>.
lordrasmus commented on code in PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#discussion_r1955038895
##########
arch/arm/src/max326xx/max32690/max32690_gpio.c:
##########
@@ -0,0 +1,448 @@
+/****************************************************************************
+ * arch/arm/src/max326xx/max32690/max32690_gpio.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/spinlock.h>
+
+#include "arm_internal.h"
+#include "max326_gpio.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+spinlock_t g_max32690_gpio_lock = SP_UNLOCKED;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_default
+ *
+ * Description:
+ * Configure a pin as a default GPIO input as it was on reset
+ *
+ ****************************************************************************/
+
+static void max32690_default(max32690_gpio_regs_t *gpio,
+ max32690_pinconfig_t *conf)
+{
+ uint32_t pinmask = 1 << conf->pin;
+
+ /* Disable interrupts and wake-up events */
+
+ gpio->inten_clr = pinmask; /* Pin interrupt disabled triggered */
+ gpio->intmode &= ~pinmask; /* Level triggered */
+ gpio->intpol &= ~pinmask; /* Input low triggers */
+ gpio->dualedge &= ~pinmask; /* Disable dual edge */
+ gpio->wken_clr = pinmask; /* Disable wakeup */
+
+ /* Make the pins an input an clear output value */
+
+ gpio->outen_clr = pinmask; /* Disable output drivers */
+ gpio->out_clr = pinmask; /* Set the output value to zero */
+
+ /* Set alternate functions to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ /* enable input connection to pin */
+
+ gpio->inen |= pinmask;
+
+ /* Reset drive strength */
+
+ gpio->ds0 &= ~pinmask;
+ gpio->ds1 &= ~pinmask;
+
+ /* Disable pull up and pull down */
+
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+
+ /* Set VVoltage to V_DDIOH */
+
+ gpio->vssel |= pinmask;
+
+ /* Disable slew and hysteresis */
+
+ gpio->hysen &= ~pinmask;
+ gpio->srsel &= ~pinmask;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_get_gpio_bank_regptr
+ *
+ * Description:
+ * Get the GPIO access struct address based in the GPIO Bank
+ *
+ ****************************************************************************/
+
+max32690_gpio_regs_t *max32690_get_gpio_bank_regptr(int bank)
+{
+ max32690_gpio_regs_t *gpio_regs = 0;
+
+ switch (bank)
+ {
+ case 0: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO0_BASE;
+ break;
+ case 1: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO1_BASE;
+ break;
+ case 2: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO2_BASE;
+ break;
+ case 3: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO3_BASE;
+ break;
+ }
+
+ DEBUGASSERT(gpio_regs != 0);
+
+ return gpio_regs;
+}
+
+static
+void set_pin_electrical_characteristics(max32690_gpio_regs_t *gpio,
+ uint16_t conf,
+ uint32_t pinmask)
+{
+ /* Enable slew and hysteresis */
+
+ if ((conf & GPIO_SLEW) != 0)
+ {
+ gpio->srsel |= pinmask;
+ }
+
+ if ((conf & GPIO_HYSTERESIS) != 0)
+ {
+ gpio->hysen |= pinmask;
+ }
+
+ switch (conf & GPIO_DRIVE_MASK)
+ {
+ case GPIO_DRIVE_0_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_0_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+
+ switch (conf & GPIO_IN_MODE_MASK)
+ {
+ case GPIO_FLOAT:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_WEAK:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_STRONG:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_WEAK:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_STRONG:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel |= ~pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+}
+
+/****************************************************************************
+ * Name: max32690_gpio_config
+ *
+ * Description:
+ * Configure a GPIO pin. Config based on bit-encoded descriptions.
+ *
+ ****************************************************************************/
+
+int max32690_gpio_config(max32690_pinconfig_t pinconf)
+{
+ irqstate_t flags;
+ uint32_t pinmask;
+
+ DEBUGASSERT(pinconf.pin <= GPIO_PINMAX);
+ pinmask = 1 << pinconf.pin;
+
+ /* Modification of all registers must be atomic */
+
+ flags = spin_lock_irqsave(&g_max32690_gpio_lock);
+
+ max32690_gpio_regs_t *gpio;
+ gpio = max32690_get_gpio_bank_regptr(pinconf.gpio_bank);
+
+ /* First, force the pin configuration to the default generic
+ * input state.
+ * So that we know we are starting from a known state.
+ */
+
+ max32690_default(gpio, &pinconf);
+
+ /* Then perform the actual pin configuration. We need only
+ * to set values that are not in the default, reset state.
+ */
+
+ /* Handle the pin function */
+
+ switch (pinconf.config & GPIO_FUNC_MASK)
+ {
+ case GPIO_IO:
+
+ /* first change to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ set_pin_electrical_characteristics(gpio, pinconf.config,
+ pinmask);
+
+ if ((pinconf.config & GPIO_OUTPUT_MODE) == GPIO_OUTPUT_MODE)
+ {
+ /* enable pin output mode */
+
+ gpio->outen_set = pinmask;
+ }
+
+ if ((pinconf.config & GPIO_VALUE_ONE) == GPIO_VALUE_ONE)
+ {
+ gpio->out_set = pinmask; /* Set output high */
+ }
+
+ break;
+
+ case GPIO_ALT1:
+
+ /* first change to I/O (transition to AF1) */
+
+ gpio->en0_set = pinmask;
+
Review Comment:
That line is intentional because modifying en0 carries a specific significance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#issuecomment-2656909259
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
No, this PR does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details and context. Here's a breakdown of what's missing:
* **Summary:**
* Missing the "Why" - Why is adding MAX32690 GPIO support necessary? Is it for a new board, supporting a new feature, or fixing a bug?
* Insufficient "How" - "a complete new driver" is too vague. What specific changes were made? What new functions were added? How does the new driver interact with existing code?
* **Impact:**
* "should have no impact" is unlikely. At minimum, the build system is affected (new files added). Be explicit about *all* impacts, even if they seem minor. Does this change affect any other drivers or subsystems?
* **Testing:**
* **Insufficient Detail:** "driver works on my dev board" is not enough. Specify the OS, CPU architecture of your build host, the compiler used, and the *exact* target configuration (board, architecture, and any specific configuration options used).
* **Missing Logs:** Provide actual logs demonstrating the functionality before and after the change. What tests were performed? Show output that proves the GPIO driver is functioning correctly. Example: Setting a GPIO high and low and verifying with a multimeter.
**Example of a better-formatted response:**
**Summary:**
This PR adds GPIO driver support for the Maxim Integrated MAX32690 microcontroller. This is necessary to enable development on boards utilizing this MCU, specifically the [Board Name] which is currently under development. This implementation provides functions for configuring and controlling the MAX32690's GPIO pins, including setting direction, output level, and reading input values. The driver utilizes the existing NuttX GPIO framework and introduces a new driver specifically for the MAX32690's unique GPIO features. It includes functions for [List key functions added, e.g., `max32690_gpio_config()`, `max32690_gpio_set()`, `max32690_gpio_read()`].
**Impact:**
* **New Feature:** Adds support for MAX32690 GPIOs.
* **Impact on User:** YES. Users will need to configure the new driver in their board configuration files to use the MAX32690 GPIOs. Documentation updates will be provided to guide users through this process.
* **Impact on Build:** YES. New source and header files are added to the build system.
* **Impact on Hardware:** YES. Support added for MAX32690. Specifically, this affects boards using this MCU.
* **Impact on Documentation:** YES. Documentation will be updated to explain the configuration and usage of the new driver. This documentation will be submitted in a separate PR.
* **Impact on Security:** NO.
* **Impact on Compatibility:** NO.
* **Anything else to consider:** None.
**Testing:**
I confirm that changes are verified on my local setup and work as intended:
* **Build Host:** Ubuntu 22.04, x86_64, GCC 11.3.0
* **Target:** sim:max32690-evkit
**Testing logs before change:**
```
(Show attempts to use GPIOs that fail because the driver isn't present)
```
**Testing logs after change:**
```
nsh> max32690_gpio_config 12 OUTPUT
nsh> max32690_gpio_set 12 1
nsh> max32690_gpio_read 12
1
nsh> max32690_gpio_set 12 0
nsh> max32690_gpio_read 12
0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#discussion_r1955145611
##########
arch/arm/src/max326xx/max32690/max32690_gpio.c:
##########
@@ -0,0 +1,448 @@
+/****************************************************************************
+ * arch/arm/src/max326xx/max32690/max32690_gpio.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/spinlock.h>
+
+#include "arm_internal.h"
+#include "max326_gpio.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+spinlock_t g_max32690_gpio_lock = SP_UNLOCKED;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_default
+ *
+ * Description:
+ * Configure a pin as a default GPIO input as it was on reset
+ *
+ ****************************************************************************/
+
+static void max32690_default(max32690_gpio_regs_t *gpio,
+ max32690_pinconfig_t *conf)
+{
+ uint32_t pinmask = 1 << conf->pin;
+
+ /* Disable interrupts and wake-up events */
+
+ gpio->inten_clr = pinmask; /* Pin interrupt disabled triggered */
+ gpio->intmode &= ~pinmask; /* Level triggered */
+ gpio->intpol &= ~pinmask; /* Input low triggers */
+ gpio->dualedge &= ~pinmask; /* Disable dual edge */
+ gpio->wken_clr = pinmask; /* Disable wakeup */
+
+ /* Make the pins an input an clear output value */
+
+ gpio->outen_clr = pinmask; /* Disable output drivers */
+ gpio->out_clr = pinmask; /* Set the output value to zero */
+
+ /* Set alternate functions to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ /* enable input connection to pin */
+
+ gpio->inen |= pinmask;
+
+ /* Reset drive strength */
+
+ gpio->ds0 &= ~pinmask;
+ gpio->ds1 &= ~pinmask;
+
+ /* Disable pull up and pull down */
+
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+
+ /* Set VVoltage to V_DDIOH */
+
+ gpio->vssel |= pinmask;
+
+ /* Disable slew and hysteresis */
+
+ gpio->hysen &= ~pinmask;
+ gpio->srsel &= ~pinmask;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: max32690_get_gpio_bank_regptr
+ *
+ * Description:
+ * Get the GPIO access struct address based in the GPIO Bank
+ *
+ ****************************************************************************/
+
+max32690_gpio_regs_t *max32690_get_gpio_bank_regptr(int bank)
+{
+ max32690_gpio_regs_t *gpio_regs = 0;
+
+ switch (bank)
+ {
+ case 0: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO0_BASE;
+ break;
+ case 1: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO1_BASE;
+ break;
+ case 2: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO2_BASE;
+ break;
+ case 3: gpio_regs = (max32690_gpio_regs_t *)MAX326_GPIO3_BASE;
+ break;
+ }
+
+ DEBUGASSERT(gpio_regs != 0);
+
+ return gpio_regs;
+}
+
+static
+void set_pin_electrical_characteristics(max32690_gpio_regs_t *gpio,
+ uint16_t conf,
+ uint32_t pinmask)
+{
+ /* Enable slew and hysteresis */
+
+ if ((conf & GPIO_SLEW) != 0)
+ {
+ gpio->srsel |= pinmask;
+ }
+
+ if ((conf & GPIO_HYSTERESIS) != 0)
+ {
+ gpio->hysen |= pinmask;
+ }
+
+ switch (conf & GPIO_DRIVE_MASK)
+ {
+ case GPIO_DRIVE_0_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_DRIVE_0_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_1_V_DIO_H:
+ gpio->ds1 &= ~pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_2_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_DRIVE_3_V_DIO_H:
+ gpio->ds1 |= pinmask;
+ gpio->ds0 |= pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+
+ switch (conf & GPIO_IN_MODE_MASK)
+ {
+ case GPIO_FLOAT:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_WEAK:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULLUP_STRONG:
+ gpio->padctrl1 &= ~pinmask;
+ gpio->padctrl0 |= pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel &= ~pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_WEAK:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps &= ~pinmask;
+ gpio->vssel |= pinmask;
+ break;
+
+ case GPIO_PULL_DOWN_STRONG:
+ gpio->padctrl1 |= pinmask;
+ gpio->padctrl0 &= ~pinmask;
+ gpio->ps |= pinmask;
+ gpio->vssel |= ~pinmask;
+ break;
+
+ default:
+ DEBUGPANIC();
+ return;
+ }
+}
+
+/****************************************************************************
+ * Name: max32690_gpio_config
+ *
+ * Description:
+ * Configure a GPIO pin. Config based on bit-encoded descriptions.
+ *
+ ****************************************************************************/
+
+int max32690_gpio_config(max32690_pinconfig_t pinconf)
+{
+ irqstate_t flags;
+ uint32_t pinmask;
+
+ DEBUGASSERT(pinconf.pin <= GPIO_PINMAX);
+ pinmask = 1 << pinconf.pin;
+
+ /* Modification of all registers must be atomic */
+
+ flags = spin_lock_irqsave(&g_max32690_gpio_lock);
+
+ max32690_gpio_regs_t *gpio;
+ gpio = max32690_get_gpio_bank_regptr(pinconf.gpio_bank);
+
+ /* First, force the pin configuration to the default generic
+ * input state.
+ * So that we know we are starting from a known state.
+ */
+
+ max32690_default(gpio, &pinconf);
+
+ /* Then perform the actual pin configuration. We need only
+ * to set values that are not in the default, reset state.
+ */
+
+ /* Handle the pin function */
+
+ switch (pinconf.config & GPIO_FUNC_MASK)
+ {
+ case GPIO_IO:
+
+ /* first change to I/O */
+
+ gpio->en0_set = pinmask;
+ gpio->en1_clr = pinmask;
+ gpio->en2_clr = pinmask;
+
+ set_pin_electrical_characteristics(gpio, pinconf.config,
+ pinmask);
+
+ if ((pinconf.config & GPIO_OUTPUT_MODE) == GPIO_OUTPUT_MODE)
+ {
+ /* enable pin output mode */
+
+ gpio->outen_set = pinmask;
+ }
+
+ if ((pinconf.config & GPIO_VALUE_ONE) == GPIO_VALUE_ONE)
+ {
+ gpio->out_set = pinmask; /* Set output high */
+ }
+
+ break;
+
+ case GPIO_ALT1:
+
+ /* first change to I/O (transition to AF1) */
+
+ gpio->en0_set = pinmask;
+
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #15837:
URL: https://github.com/apache/nuttx/pull/15837#issuecomment-2657603478
@lordrasmus please fix these errors:
```
In file included from /github/workspace/sources/nuttx/arch/arm/src/chip/max326_gpio.h:40,
from chip/common/max326_irq.c:42:
Error: /github/workspace/sources/nuttx/arch/arm/src/chip/max32660/max32660_gpio.h:244:24: error: unknown type name 'max326_pinconfig_t'; did you mean 'max326_pinset_t'?
244 | void max326_gpio_write(max326_pinconfig_t pinset, bool value);
| ^~~~~~~~~~~~~~~~~~
| max326_pinset_t
make[1]: *** [Makefile:170: max326_irq.o] Error 1
In file included from /github/workspace/sources/nuttx/arch/arm/src/chip/max326_gpio.h:40,
from chip/max32660/max32660_lowputc.c:43:
Error: /github/workspace/sources/nuttx/arch/arm/src/chip/max32660/max32660_gpio.h:244:24: error: unknown type name 'max326_pinconfig_t'; did you mean 'max326_pinset_t'?
244 | void max326_gpio_write(max326_pinconfig_t pinset, bool value);
| ^~~~~~~~~~~~~~~~~~
| max326_pinset_t
make[1]: *** [Makefile:170: max32660_lowputc.o] Error 1
In file included from /github/workspace/sources/nuttx/arch/arm/src/chip/max326_gpio.h:40,
from chip/max32660/max32660_gpio.c:37:
Error: /github/workspace/sources/nuttx/arch/arm/src/chip/max32660/max32660_gpio.h:244:24: error: unknown type name 'max326_pinconfig_t'; did you mean 'max326_pinset_t'?
244 | void max326_gpio_write(max326_pinconfig_t pinset, bool value);
| ^~~~~~~~~~~~~~~~~~
| max326_pinset_t
make[1]: *** [Makefile:170: max32660_gpio.o] Error 1
make[1]: Target 'libarch.a' not remade because of errors.
make: *** [tools/LibTargets.mk:170: arch/arm/src/libarch.a] Error 2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] arch/arm/max326xx: add max32690 gpio driver [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #15837:
URL: https://github.com/apache/nuttx/pull/15837
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org