michallenc commented on code in PR #21516: URL: https://github.com/apache/nuttx/pull/21516#discussion_r4505725527
########## arch/arm/src/imxrt/imxrt_adc_ver3.c: ########## @@ -0,0 +1,413 @@ +/**************************************************************************** + * arch/arm/src/imxrt/imxrt_adc_ver3.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 <errno.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/analog/adc.h> +#include <nuttx/analog/ioctl.h> +#include <nuttx/arch.h> +#include <nuttx/irq.h> + +#include "arm_internal.h" +#include "chip.h" +#include "hardware/imxrt_adc.h" +#include "imxrt_periphclks.h" + +#ifdef CONFIG_IMXRT_ADC +#if defined(CONFIG_IMXRT_ADC1) || defined(CONFIG_IMXRT_ADC2) + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ +#define ADC_MAX_CHANNELS 16 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct imxrt_adc_ver3_s +{ + const struct adc_callback_s *cb; + uint8_t intf; + uint32_t base; + int irq; + uint8_t initialized; + uint8_t nchannels; + uint8_t chanlist[ADC_MAX_CHANNELS]; + uint8_t current; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback); +static void adc_reset(struct adc_dev_s *dev); +static int adc_setup(struct adc_dev_s *dev); +static void adc_shutdown(struct adc_dev_s *dev); +static void adc_rxint(struct adc_dev_s *dev, bool enable); +static int adc_ioctl(struct adc_dev_s *dev, int cmd, unsigned long arg); +static int adc_interrupt(int irq, void *context, void *arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct adc_ops_s g_adcops = +{ + .ao_bind = adc_bind, + .ao_reset = adc_reset, + .ao_setup = adc_setup, + .ao_shutdown = adc_shutdown, + .ao_rxint = adc_rxint, + .ao_ioctl = adc_ioctl, +}; + +#ifdef CONFIG_IMXRT_ADC1 +static struct imxrt_adc_ver3_s g_adcpriv1 = +{ + .intf = 1, + .base = IMXRT_ADC1_BASE, + .irq = IMXRT_IRQ_ADC1 +}; +static struct adc_dev_s g_adcdev1 = +{ + &g_adcops, + &g_adcpriv1 +}; +#endif + +#ifdef CONFIG_IMXRT_ADC2 +static struct imxrt_adc_ver3_s g_adcpriv2 = +{ + .intf = 2, + .base = IMXRT_ADC2_BASE, + .irq = IMXRT_IRQ_ADC2 +}; +static struct adc_dev_s g_adcdev2 = +{ + &g_adcops, + &g_adcpriv2 +}; +#endif + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static inline uint32_t adc_getreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset) +{ + return getreg32(priv->base + offset); +} + +static inline void adc_putreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset, uint32_t value) +{ + putreg32(value, priv->base + offset); +} + +/**************************************************************************** + * Name: adc_bind + * + * Description: + * Bind the upper-half driver callbacks to the lower-half implementation. + * This must be called early in order to receive ADC event notifications. + * + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback) +{ + ((struct imxrt_adc_ver3_s *)dev->ad_priv)->cb = callback; + return OK; +} + +/**************************************************************************** + * Name: adc_reset + * + * Description: + * Reset the ADC device. Called early to initialize the hardware. This + * is called, before adc_setup() and on error conditions. + * + ****************************************************************************/ + +static void adc_reset(struct adc_dev_s *dev) +{ + struct imxrt_adc_ver3_s *priv = dev->ad_priv; + + if (priv->initialized != 0) + { + return; + } + + if (priv->intf == 1) + { + imxrt_clockall_adc1(); + } + else + { + imxrt_clockall_adc2(); + } + + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, IMXRT_ADC_CTRL_RST); + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, 0); + adc_putreg(priv, IMXRT_ADC_CFG_OFFSET, IMXRT_ADC_CFG_PWREN | + (0x10u << IMXRT_ADC_CFG_PUDLY_SHIFT)); + adc_putreg(priv, IMXRT_ADC_FCTRL0_OFFSET, IMXRT_ADC_FCTRL_FWMARK(0)); + adc_putreg(priv, IMXRT_ADC_IE_OFFSET, 0); + + /* Keep calibration out of the reset path until the gain-result sequence + * (GCC/GCR) is implemented. The SDK examples initialize this way too. + */ + + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, IMXRT_ADC_CTRL_ADCEN); +} + +static void adc_program(struct imxrt_adc_ver3_s *priv, uint8_t channel) Review Comment: nitpick, but please add a documentation what the function does - similar to other functions. ########## arch/arm/src/imxrt/imxrt_adc_ver3.c: ########## @@ -0,0 +1,413 @@ +/**************************************************************************** + * arch/arm/src/imxrt/imxrt_adc_ver3.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 <errno.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/analog/adc.h> +#include <nuttx/analog/ioctl.h> +#include <nuttx/arch.h> +#include <nuttx/irq.h> + +#include "arm_internal.h" +#include "chip.h" +#include "hardware/imxrt_adc.h" +#include "imxrt_periphclks.h" + +#ifdef CONFIG_IMXRT_ADC +#if defined(CONFIG_IMXRT_ADC1) || defined(CONFIG_IMXRT_ADC2) + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ +#define ADC_MAX_CHANNELS 16 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct imxrt_adc_ver3_s +{ + const struct adc_callback_s *cb; + uint8_t intf; + uint32_t base; + int irq; + uint8_t initialized; + uint8_t nchannels; + uint8_t chanlist[ADC_MAX_CHANNELS]; + uint8_t current; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback); +static void adc_reset(struct adc_dev_s *dev); +static int adc_setup(struct adc_dev_s *dev); +static void adc_shutdown(struct adc_dev_s *dev); +static void adc_rxint(struct adc_dev_s *dev, bool enable); +static int adc_ioctl(struct adc_dev_s *dev, int cmd, unsigned long arg); +static int adc_interrupt(int irq, void *context, void *arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct adc_ops_s g_adcops = +{ + .ao_bind = adc_bind, + .ao_reset = adc_reset, + .ao_setup = adc_setup, + .ao_shutdown = adc_shutdown, + .ao_rxint = adc_rxint, + .ao_ioctl = adc_ioctl, +}; + +#ifdef CONFIG_IMXRT_ADC1 +static struct imxrt_adc_ver3_s g_adcpriv1 = +{ + .intf = 1, + .base = IMXRT_ADC1_BASE, + .irq = IMXRT_IRQ_ADC1 +}; +static struct adc_dev_s g_adcdev1 = +{ + &g_adcops, + &g_adcpriv1 +}; +#endif + +#ifdef CONFIG_IMXRT_ADC2 +static struct imxrt_adc_ver3_s g_adcpriv2 = +{ + .intf = 2, + .base = IMXRT_ADC2_BASE, + .irq = IMXRT_IRQ_ADC2 +}; +static struct adc_dev_s g_adcdev2 = +{ + &g_adcops, + &g_adcpriv2 +}; +#endif + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static inline uint32_t adc_getreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset) +{ + return getreg32(priv->base + offset); +} + +static inline void adc_putreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset, uint32_t value) +{ + putreg32(value, priv->base + offset); +} + +/**************************************************************************** + * Name: adc_bind + * + * Description: + * Bind the upper-half driver callbacks to the lower-half implementation. + * This must be called early in order to receive ADC event notifications. + * + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback) +{ + ((struct imxrt_adc_ver3_s *)dev->ad_priv)->cb = callback; + return OK; +} + +/**************************************************************************** + * Name: adc_reset + * + * Description: + * Reset the ADC device. Called early to initialize the hardware. This + * is called, before adc_setup() and on error conditions. + * + ****************************************************************************/ + +static void adc_reset(struct adc_dev_s *dev) +{ + struct imxrt_adc_ver3_s *priv = dev->ad_priv; + + if (priv->initialized != 0) + { + return; + } + + if (priv->intf == 1) + { + imxrt_clockall_adc1(); + } + else + { + imxrt_clockall_adc2(); + } + + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, IMXRT_ADC_CTRL_RST); + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, 0); + adc_putreg(priv, IMXRT_ADC_CFG_OFFSET, IMXRT_ADC_CFG_PWREN | + (0x10u << IMXRT_ADC_CFG_PUDLY_SHIFT)); + adc_putreg(priv, IMXRT_ADC_FCTRL0_OFFSET, IMXRT_ADC_FCTRL_FWMARK(0)); + adc_putreg(priv, IMXRT_ADC_IE_OFFSET, 0); + + /* Keep calibration out of the reset path until the gain-result sequence + * (GCC/GCR) is implemented. The SDK examples initialize this way too. + */ + + adc_putreg(priv, IMXRT_ADC_CTRL_OFFSET, IMXRT_ADC_CTRL_ADCEN); +} + +static void adc_program(struct imxrt_adc_ver3_s *priv, uint8_t channel) +{ + uint8_t hardware_channel = channel; + uint32_t channel_type = IMXRT_ADC_CMDL_CTYPE_A; + + /* IDs 0-7 are A0-A7; IDs 8-15 are B0-B7. */ + + if (channel >= 8) + { + hardware_channel = channel - 8; + channel_type = IMXRT_ADC_CMDL_CTYPE_B; + } + + uint32_t cmdl = IMXRT_ADC_CMDL_ADCH(hardware_channel) | + IMXRT_ADC_CMDL_CTYPE(channel_type) | + IMXRT_ADC_CMDL_CSCALE(1); + + adc_putreg(priv, IMXRT_ADC_CMDL1_OFFSET, cmdl); + adc_putreg(priv, IMXRT_ADC_CMDH1_OFFSET, 0); + adc_putreg(priv, IMXRT_ADC_TCTRL0_OFFSET, IMXRT_ADC_TCTRL_TCMD(1)); +} + +/**************************************************************************** + * Name: adc_setup + * + * Description: + * Configure the ADC. This method is called the first time that the ADC + * device is opened. This will occur when the port is first opened. + * This setup includes configuring and attaching ADC interrupts. + * Interrupts are all disabled upon return. + * + ****************************************************************************/ + +static int adc_setup(struct adc_dev_s *dev) +{ + struct imxrt_adc_ver3_s *priv = dev->ad_priv; + int ret; + + if (priv->initialized++ != 0) + { + return OK; + } + + priv->current = 0; + adc_program(priv, priv->chanlist[0]); Review Comment: Why setup only the first channel? ########## arch/arm/src/imxrt/imxrt_adc_ver3.c: ########## @@ -0,0 +1,413 @@ +/**************************************************************************** + * arch/arm/src/imxrt/imxrt_adc_ver3.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 <errno.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/analog/adc.h> +#include <nuttx/analog/ioctl.h> +#include <nuttx/arch.h> +#include <nuttx/irq.h> + +#include "arm_internal.h" +#include "chip.h" +#include "hardware/imxrt_adc.h" +#include "imxrt_periphclks.h" + +#ifdef CONFIG_IMXRT_ADC +#if defined(CONFIG_IMXRT_ADC1) || defined(CONFIG_IMXRT_ADC2) + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ +#define ADC_MAX_CHANNELS 16 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct imxrt_adc_ver3_s +{ + const struct adc_callback_s *cb; + uint8_t intf; + uint32_t base; + int irq; + uint8_t initialized; + uint8_t nchannels; + uint8_t chanlist[ADC_MAX_CHANNELS]; + uint8_t current; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback); +static void adc_reset(struct adc_dev_s *dev); +static int adc_setup(struct adc_dev_s *dev); +static void adc_shutdown(struct adc_dev_s *dev); +static void adc_rxint(struct adc_dev_s *dev, bool enable); +static int adc_ioctl(struct adc_dev_s *dev, int cmd, unsigned long arg); +static int adc_interrupt(int irq, void *context, void *arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct adc_ops_s g_adcops = +{ + .ao_bind = adc_bind, + .ao_reset = adc_reset, + .ao_setup = adc_setup, + .ao_shutdown = adc_shutdown, + .ao_rxint = adc_rxint, + .ao_ioctl = adc_ioctl, +}; + +#ifdef CONFIG_IMXRT_ADC1 +static struct imxrt_adc_ver3_s g_adcpriv1 = +{ + .intf = 1, + .base = IMXRT_ADC1_BASE, + .irq = IMXRT_IRQ_ADC1 +}; +static struct adc_dev_s g_adcdev1 = +{ + &g_adcops, + &g_adcpriv1 +}; +#endif + +#ifdef CONFIG_IMXRT_ADC2 +static struct imxrt_adc_ver3_s g_adcpriv2 = +{ + .intf = 2, + .base = IMXRT_ADC2_BASE, + .irq = IMXRT_IRQ_ADC2 +}; +static struct adc_dev_s g_adcdev2 = +{ + &g_adcops, + &g_adcpriv2 +}; +#endif + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static inline uint32_t adc_getreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset) +{ + return getreg32(priv->base + offset); +} + +static inline void adc_putreg(struct imxrt_adc_ver3_s *priv, + uint32_t offset, uint32_t value) +{ + putreg32(value, priv->base + offset); +} + +/**************************************************************************** + * Name: adc_bind + * + * Description: + * Bind the upper-half driver callbacks to the lower-half implementation. + * This must be called early in order to receive ADC event notifications. + * + ****************************************************************************/ + +static int adc_bind(struct adc_dev_s *dev, + const struct adc_callback_s *callback) +{ + ((struct imxrt_adc_ver3_s *)dev->ad_priv)->cb = callback; + return OK; +} + +/**************************************************************************** + * Name: adc_reset + * + * Description: + * Reset the ADC device. Called early to initialize the hardware. This + * is called, before adc_setup() and on error conditions. + * + ****************************************************************************/ + +static void adc_reset(struct adc_dev_s *dev) +{ + struct imxrt_adc_ver3_s *priv = dev->ad_priv; + + if (priv->initialized != 0) + { + return; + } + + if (priv->intf == 1) + { + imxrt_clockall_adc1(); + } + else + { + imxrt_clockall_adc2(); + } + Review Comment: I don't see GPIO definition and configuration anywhere. It should be here in `adc_reset` or in `adc_setup`. You can take a look at ver1 and ver2 drivers, something like ```c gpio_pinset_t pinset = 0; for (int i = 0; i < priv->nchannels; i++) { DEBUGASSERT(priv->chanlist[i] < ADC_MAX_CHANNELS); pinset = pinlist[priv->chanlist[i]] | IOMUX_ADC_DEFAULT; imxrt_config_gpio(pinset); } ``` Otherwise the pins aren't correctly defined and driver won't work. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
