raulcxw commented on code in PR #19484: URL: https://github.com/apache/nuttx/pull/19484#discussion_r3619023752
########## arch/arm/src/common/ameba/ameba_gpio.c: ########## @@ -0,0 +1,555 @@ +/**************************************************************************** + * arch/arm/src/common/ameba/ameba_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 + ****************************************************************************/ + +/* NuttX GPIO (ioexpander) lower half for the Realtek Ameba GPIO controller. + * + * The pad mux, pull resistors and GPIO port registers are all programmed + * through the SDK fwlib GPIO API (GPIO_Init(), GPIO_WriteBit(), ...). + * + * Most of the API has no SDK source and resolves to the on-chip ROM symbol + * table (GPIO_WriteBit, GPIO_ReadDataBit, GPIO_INTConfig, GPIO_Direction, + * GPIO_INTMode, Pinmux_Config, PAD_PullCtrl, ...). Only GPIO_INTStatusGet + * and GPIO_INTStatusClearEdge are missing from ROM; they live in fwlib + * ram_common/ameba_gpio.c, compiled into libameba_fwlib.a to provide them + * (see AMEBA_FWLIB_SRCS). That same source also defines GPIO_Init (which + * IS in ROM), so the local object overrides the ROM PROVIDE(GPIO_Init) as a + * harmless side effect -- same source, equivalent behaviour. To keep the + * vendor headers out of the NuttX include world, the handful of fwlib + * symbols and the GPIO_InitTypeDef layout used here are declared locally + * below rather than pulled in from <ameba_gpio.h>. + * + * Interrupt dispatch stays NuttX-native: the port's NVIC vector is owned by + * NuttX (irq_attach), and the ISR reads/clears the controller status through + * the fwlib GPIO_INTStatus* helpers before calling each pending pin's + * registered callback. + */ + +#include <nuttx/config.h> + +#include <stdint.h> +#include <stdbool.h> +#include <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/kmalloc.h> +#include <nuttx/spinlock.h> +#include <nuttx/ioexpander/gpio.h> + +#include "arm_internal.h" +#include "ameba_gpio.h" + +/* Per-chip GPIO parameters (port count, per-port NVIC vectors, RCC gate + * bits). Resolved from the configured chip's directory on the include path + * (arch/.../chip -> arch/arm/src/<chip>); see <ameba_gpio_chip.h> there. + */ + +#include "ameba_gpio_chip.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* Pin encoding (SDK "PinName": port in bits[7:5], pin num in bits[4:0]). */ + +#define AMEBA_PIN_PORT(pin) (((pin) >> 5) & 0x07) +#define AMEBA_PIN_NUM(pin) ((pin) & 0x1f) + +/* Mirror of the fwlib GPIO_InitTypeDef field values (ameba_gpio.h). + * Declared locally so the SDK header need not leak into NuttX includes. + */ + +#define AMEBA_GPIO_MODE_IN 0x0 /* GPIO_Mode_IN */ +#define AMEBA_GPIO_MODE_OUT 0x1 /* GPIO_Mode_OUT */ +#define AMEBA_GPIO_MODE_INT 0x2 /* GPIO_Mode_INT */ + +#define AMEBA_GPIO_PUPD_NONE 0x0 /* GPIO_PuPd_NOPULL */ +#define AMEBA_GPIO_PUPD_DOWN 0x1 /* GPIO_PuPd_DOWN */ +#define AMEBA_GPIO_PUPD_UP 0x2 /* GPIO_PuPd_UP */ + +#define AMEBA_GPIO_IT_LEVEL 0x0 /* GPIO_INT_Trigger_LEVEL */ +#define AMEBA_GPIO_IT_EDGE 0x1 /* GPIO_INT_Trigger_EDGE */ +#define AMEBA_GPIO_IT_BOTHEDGE 0x2 /* GPIO_INT_Trigger_BOTHEDGE */ + +#define AMEBA_GPIO_POL_LOW 0x0 /* GPIO_INT_POLARITY_ACTIVE_LOW */ +#define AMEBA_GPIO_POL_HIGH 0x1 /* GPIO_INT_POLARITY_ACTIVE_HIGH */ + +#define AMEBA_GPIO_DEBOUNCE_OFF 0x0 /* GPIO_INT_DEBOUNCE_DISABLE */ + +/* Second argument to GPIO_INTConfig() / state passed to GPIO_WriteBit(). */ + +#define AMEBA_DISABLE 0x0 +#define AMEBA_ENABLE 0x1 + +/* AMEBA_APBPERIPH_GPIO (the RCC gate bits), AMEBA_GPIO_NPORTS and the + * AMEBA_GPIO_PORT_IRQS vector table come from <ameba_gpio_chip.h>. The + * fwlib GPIO_Init() leaves the peripheral clock to the caller, so the driver + * gates AMEBA_APBPERIPH_GPIO on before use (RCC touches the LSYS block). + */ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* Layout-compatible mirror of the fwlib GPIO_InitTypeDef (all u32, same + * order); passed by address to GPIO_Init(). + */ + +struct ameba_gpio_init_s +{ + uint32_t mode; /* GPIO_Mode */ + uint32_t pupd; /* GPIO_PuPd */ + uint32_t ittrigger; /* GPIO_ITTrigger */ + uint32_t itpolarity; /* GPIO_ITPolarity */ + uint32_t itdebounce; /* GPIO_ITDebounce */ + uint32_t pin; /* GPIO_Pin */ +}; + +struct ameba_gpio_dev_s +{ + struct gpio_dev_s gpio; /* Lower-half GPIO device (must be first) */ + uint8_t pin; /* SDK pin encoding (port[7:5] | num[4:0]) */ + pin_interrupt_t callback; /* Interrupt callback (interrupt pins only) */ +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +/* SDK fwlib GPIO API used by this driver. These are plain extern symbols; + * the driver does not care where each comes from -- every one is resolved + * at link time either by the on-chip ROM symbol table or by a fwlib object + * compiled into libameba_fwlib.a. Which source applies is per-chip: + * + * RCC_PeriphClockCmd, GPIO_Init, GPIO_WriteBit, GPIO_ReadDataBit and + * GPIO_INTConfig are in ROM on every Ameba ARM chip. (GPIO_Init also + * reaches Pinmux_Config, GPIO_Direction, GPIO_INTMode and PAD_PullCtrl + * internally, resolved inside ROM, so the driver never names them.) + * + * GPIO_INTStatusGet and GPIO_INTStatusClearEdge are the gap. On RTL8721Dx + * they come from fwlib ram_common/ameba_gpio.c, compiled in via + * AMEBA_FWLIB_SRCS (that object also carries GPIO_Init and harmlessly + * overrides the ROM copy). On amebalite/amebasmart they are in neither ROM + * nor any SDK source, so a chip adding those parts MUST supply its own + * definition -- a small object writing the port's GPIO_INT_STATUS / + * GPIO_INT_EOI -- via its board build. The shared driver needs no change; + * this is the per-chip contract for these two symbols. + */ + +extern void RCC_PeriphClockCmd(uint32_t periph, uint32_t clock, + uint8_t newstate); +extern void GPIO_Init(struct ameba_gpio_init_s *init); +extern void GPIO_WriteBit(uint32_t pin, uint32_t state); +extern uint32_t GPIO_ReadDataBit(uint32_t pin); +extern void GPIO_INTConfig(uint32_t pin, uint32_t newstate); +extern uint32_t GPIO_INTStatusGet(uint32_t port); +extern void GPIO_INTStatusClearEdge(uint32_t port); + +/* GPIO lower-half operations */ + +static int ameba_gpio_read(struct gpio_dev_s *dev, bool *value); +static int ameba_gpio_write(struct gpio_dev_s *dev, bool value); +static int ameba_gpio_attach(struct gpio_dev_s *dev, + pin_interrupt_t callback); +static int ameba_gpio_enable(struct gpio_dev_s *dev, bool enable); +static int ameba_gpio_setpintype(struct gpio_dev_s *dev, + enum gpio_pintype_e pintype); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct gpio_operations_s g_ameba_gpio_ops = +{ + .go_read = ameba_gpio_read, + .go_write = ameba_gpio_write, + .go_attach = ameba_gpio_attach, + .go_enable = ameba_gpio_enable, + .go_setpintype = ameba_gpio_setpintype, +}; + +/* Per-pin device lookup for interrupt dispatch, indexed [port][pin-num]. + * Set once an interrupt pin is registered; read by the port ISR. + */ + +static struct ameba_gpio_dev_s *g_int_dev[AMEBA_GPIO_NPORTS][32]; + +/* Set once each port's NuttX interrupt vector has been attached + enabled. */ + +static bool g_port_attached[AMEBA_GPIO_NPORTS]; Review Comment: Thanks for the review! Updated in the latest push: - Globals → one controller struct. These four are driver-wide / per-port, not per-pin: one NVIC vector per port drives up to 32 pins through a shared dispatch table guarded by a single spinlock, so they cannot live in the per-pin ameba_gpio_dev_s. Consolidated them into a single controller instance: struct ameba_gpio_ctrl_s { struct ameba_gpio_dev_s *int_dev[AMEBA_GPIO_NPORTS][32]; /* dispatch table */ bool port_attached[AMEBA_GPIO_NPORTS]; const int irq[AMEBA_GPIO_NPORTS]; /* per-port NVIC */ spinlock_t lock; }; static struct ameba_gpio_ctrl_s g_ameba_gpio = { .irq = AMEBA_GPIO_PORT_IRQS, .lock = SP_UNLOCKED, }; - rtl8721dx_gpio.c loop index → size_t, dropped the (int)nitems(...) cast and switched the log to %zu. Rebuilt on both make and cmake and re-ran the style checks (nxstyle / checkpatch) — all green. -- 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]
