A GPIO driver in Rust

submited by
Style Pass
2021-07-19 20:00:08

C versionRust version 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426// SPDX-License-Identifier: GPL-2.0-only /* * Copyright (C) 2008, 2009 Provigent Ltd. * * Author: Baruch Siach <baruch@tkos.co.il> * * Driver for the ARM PrimeCell(tm) General Purpose Input/Output (PL061) * * Data sheet: ARM DDI 0190B, September 2000 */ #include <linux/spinlock.h> #include <linux/errno.h> #include <linux/init.h> #include <linux/io.h> #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/irq.h> #include <linux/irqchip/chained_irq.h> #include <linux/module.h> #include <linux/bitops.h> #include <linux/gpio/driver.h> #include <linux/device.h> #include <linux/amba/bus.h> #include <linux/slab.h> #include <linux/pinctrl/consumer.h> #include <linux/pm.h> #define GPIODIR 0x400 #define GPIOIS 0x404 #define GPIOIBE 0x408 #define GPIOIEV 0x40C #define GPIOIE 0x410 #define GPIORIS 0x414 #define GPIOMIS 0x418 #define GPIOIC 0x41C #define PL061_GPIO_NR 8 #ifdef CONFIG_PM struct pl061_context_save_regs { u8 gpio_data; u8 gpio_dir; u8 gpio_is; u8 gpio_ibe; u8 gpio_iev; u8 gpio_ie; }; #endif struct pl061 { raw_spinlock_t lock; void __iomem *base; struct gpio_chip gc; struct irq_chip irq_chip; int parent_irq; #ifdef CONFIG_PM struct pl061_context_save_regs csave_regs; #endif }; static int pl061_get_direction(struct gpio_chip *gc, unsigned offset) { struct pl061 *pl061 = gpiochip_get_data(gc); if (readb(pl061->base + GPIODIR) & BIT(offset)) return GPIO_LINE_DIRECTION_OUT; return GPIO_LINE_DIRECTION_IN; } static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) { struct pl061 *pl061 = gpiochip_get_data(gc); unsigned long flags; unsigned char gpiodir; raw_spin_lock_irqsave(&pl061->lock, flags); gpiodir = readb(pl061->base + GPIODIR); gpiodir &= ~(BIT(offset)); writeb(gpiodir, pl061->base + GPIODIR); raw_spin_unlock_irqrestore(&pl061->lock, flags); return 0; } static int pl061_direction_output(struct gpio_chip *gc, unsigned offset, int value) { struct pl061 *pl061 = gpiochip_get_data(gc); unsigned long flags; unsigned char gpiodir; raw_spin_lock_irqsave(&pl061->lock, flags); writeb(!!value << offset, pl061->base + (BIT(offset + 2))); gpiodir = readb(pl061->base + GPIODIR); gpiodir |= BIT(offset); writeb(gpiodir, pl061->base + GPIODIR); /* * gpio value is set again, because pl061 doesn't allow to set value of * a gpio pin before configuring it in OUT mode. */ writeb(!!value << offset, pl061->base + (BIT(offset + 2))); raw_spin_unlock_irqrestore(&pl061->lock, flags); return 0; } static int pl061_get_value(struct gpio_chip *gc, unsigned offset) { struct pl061 *pl061 = gpiochip_get_data(gc); return !!readb(pl061->base + (BIT(offset + 2))); } static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value) { struct pl061 *pl061 = gpiochip_get_data(gc); writeb(!!value << offset, pl061->base + (BIT(offset + 2))); } static void pl061_irq_handler(struct irq_desc *desc) { unsigned long pending; int offset; struct gpio_chip *gc = irq_desc_get_handler_data(desc); struct pl061 *pl061 = gpiochip_get_data(gc); struct irq_chip *irqchip = irq_desc_get_chip(desc); chained_irq_enter(irqchip, desc); pending = readb(pl061->base + GPIOMIS); if (pending) { for_each_set_bit(offset, &pending, PL061_GPIO_NR) generic_handle_irq(irq_find_mapping(gc->irq.domain, offset)); } chained_irq_exit(irqchip, desc); } static int pl061_irq_type(struct irq_data *d, unsigned trigger) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pl061 *pl061 = gpiochip_get_data(gc); int offset = irqd_to_hwirq(d); unsigned long flags; u8 gpiois, gpioibe, gpioiev; u8 bit = BIT(offset); if (offset < 0 || offset >= PL061_GPIO_NR) return -EINVAL; if ((trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) && (trigger & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))) { dev_err(gc->parent, "trying to configure line %d for both level and edge " "detection, choose one!\n", offset); return -EINVAL; } raw_spin_lock_irqsave(&pl061->lock, flags); gpioiev = readb(pl061->base + GPIOIEV); gpiois = readb(pl061->base + GPIOIS); gpioibe = readb(pl061->base + GPIOIBE); if (trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) { bool polarity = trigger & IRQ_TYPE_LEVEL_HIGH; /* Disable edge detection */ gpioibe &= ~bit; /* Enable level detection */ gpiois |= bit; /* Select polarity */ if (polarity) gpioiev |= bit; else gpioiev &= ~bit; irq_set_handler_locked(d, handle_level_irq); dev_dbg(gc->parent, "line %d: IRQ on %s level\n", offset, polarity ? "HIGH" : "LOW"); } else if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) { /* Disable level detection */ gpiois &= ~bit; /* Select both edges, setting this makes GPIOEV be ignored */ gpioibe |= bit; irq_set_handler_locked(d, handle_edge_irq); dev_dbg(gc->parent, "line %d: IRQ on both edges\n", offset); } else if ((trigger & IRQ_TYPE_EDGE_RISING) || (trigger & IRQ_TYPE_EDGE_FALLING)) { bool rising = trigger & IRQ_TYPE_EDGE_RISING; /* Disable level detection */ gpiois &= ~bit; /* Clear detection on both edges */ gpioibe &= ~bit; /* Select edge */ if (rising) gpioiev |= bit; else gpioiev &= ~bit; irq_set_handler_locked(d, handle_edge_irq); dev_dbg(gc->parent, "line %d: IRQ on %s edge\n", offset, rising ? "RISING" : "FALLING"); } else { /* No trigger: disable everything */ gpiois &= ~bit; gpioibe &= ~bit; gpioiev &= ~bit; irq_set_handler_locked(d, handle_bad_irq); dev_warn(gc->parent, "no trigger selected for line %d\n", offset); } writeb(gpiois, pl061->base + GPIOIS); writeb(gpioibe, pl061->base + GPIOIBE); writeb(gpioiev, pl061->base + GPIOIEV); raw_spin_unlock_irqrestore(&pl061->lock, flags); return 0; } static void pl061_irq_mask(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pl061 *pl061 = gpiochip_get_data(gc); u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR); u8 gpioie; raw_spin_lock(&pl061->lock); gpioie = readb(pl061->base + GPIOIE) & ~mask; writeb(gpioie, pl061->base + GPIOIE); raw_spin_unlock(&pl061->lock); } static void pl061_irq_unmask(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pl061 *pl061 = gpiochip_get_data(gc); u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR); u8 gpioie; raw_spin_lock(&pl061->lock); gpioie = readb(pl061->base + GPIOIE) | mask; writeb(gpioie, pl061->base + GPIOIE); raw_spin_unlock(&pl061->lock); } /** * pl061_irq_ack() - ACK an edge IRQ * @d: IRQ data for this IRQ * * This gets called from the edge IRQ handler to ACK the edge IRQ * in the GPIOIC (interrupt-clear) register. For level IRQs this is * not needed: these go away when the level signal goes away. */ static void pl061_irq_ack(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pl061 *pl061 = gpiochip_get_data(gc); u8 mask = BIT(irqd_to_hwirq(d) % PL061_GPIO_NR); raw_spin_lock(&pl061->lock); writeb(mask, pl061->base + GPIOIC); raw_spin_unlock(&pl061->lock); } static int pl061_irq_set_wake(struct irq_data *d, unsigned int state) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pl061 *pl061 = gpiochip_get_data(gc); return irq_set_irq_wake(pl061->parent_irq, state); } static int pl061_probe(struct amba_device *adev, const struct amba_id *id) { struct device *dev = &adev->dev; struct pl061 *pl061; struct gpio_irq_chip *girq; int ret, irq; pl061 = devm_kzalloc(dev, sizeof(*pl061), GFP_KERNEL); if (pl061 == NULL) return -ENOMEM; pl061->base = devm_ioremap_resource(dev, &adev->res); if (IS_ERR(pl061->base)) return PTR_ERR(pl061->base); raw_spin_lock_init(&pl061->lock); pl061->gc.request = gpiochip_generic_request; pl061->gc.free = gpiochip_generic_free; pl061->gc.base = -1; pl061->gc.get_direction = pl061_get_direction; pl061->gc.direction_input = pl061_direction_input; pl061->gc.direction_output = pl061_direction_output; pl061->gc.get = pl061_get_value; pl061->gc.set = pl061_set_value; pl061->gc.ngpio = PL061_GPIO_NR; pl061->gc.label = dev_name(dev); pl061->gc.parent = dev; pl061->gc.owner = THIS_MODULE; /* * irq_chip support */ pl061->irq_chip.name = dev_name(dev); pl061->irq_chip.irq_ack = pl061_irq_ack; pl061->irq_chip.irq_mask = pl061_irq_mask; pl061->irq_chip.irq_unmask = pl061_irq_unmask; pl061->irq_chip.irq_set_type = pl061_irq_type; pl061->irq_chip.irq_set_wake = pl061_irq_set_wake; writeb(0, pl061->base + GPIOIE); /* disable irqs */ irq = adev->irq[0]; if (!irq) dev_warn(&adev->dev, "IRQ support disabled\n"); pl061->parent_irq = irq; girq = &pl061->gc.irq; girq->chip = &pl061->irq_chip; girq->parent_handler = pl061_irq_handler; girq->num_parents = 1; girq->parents = devm_kcalloc(dev, 1, sizeof(*girq->parents), GFP_KERNEL); if (!girq->parents) return -ENOMEM; girq->parents[0] = irq; girq->default_type = IRQ_TYPE_NONE; girq->handler = handle_bad_irq; ret = devm_gpiochip_add_data(dev, &pl061->gc, pl061); if (ret) return ret; amba_set_drvdata(adev, pl061); dev_info(dev, "PL061 GPIO chip registered\n"); return 0; } #ifdef CONFIG_PM static int pl061_suspend(struct device *dev) { struct pl061 *pl061 = dev_get_drvdata(dev); int offset; pl061->csave_regs.gpio_data = 0; pl061->csave_regs.gpio_dir = readb(pl061->base + GPIODIR); pl061->csave_regs.gpio_is = readb(pl061->base + GPIOIS); pl061->csave_regs.gpio_ibe = readb(pl061->base + GPIOIBE); pl061->csave_regs.gpio_iev = readb(pl061->base + GPIOIEV); pl061->csave_regs.gpio_ie = readb(pl061->base + GPIOIE); for (offset = 0; offset < PL061_GPIO_NR; offset++) { if (pl061->csave_regs.gpio_dir & (BIT(offset))) pl061->csave_regs.gpio_data |= pl061_get_value(&pl061->gc, offset) << offset; } return 0; } static int pl061_resume(struct device *dev) { struct pl061 *pl061 = dev_get_drvdata(dev); int offset; for (offset = 0; offset < PL061_GPIO_NR; offset++) { if (pl061->csave_regs.gpio_dir & (BIT(offset))) pl061_direction_output(&pl061->gc, offset, pl061->csave_regs.gpio_data & (BIT(offset))); else pl061_direction_input(&pl061->gc, offset); } writeb(pl061->csave_regs.gpio_is, pl061->base + GPIOIS); writeb(pl061->csave_regs.gpio_ibe, pl061->base + GPIOIBE); writeb(pl061->csave_regs.gpio_iev, pl061->base + GPIOIEV); writeb(pl061->csave_regs.gpio_ie, pl061->base + GPIOIE); return 0; } static const struct dev_pm_ops pl061_dev_pm_ops = { .suspend = pl061_suspend, .resume = pl061_resume, .freeze = pl061_suspend, .restore = pl061_resume, }; #endif static const struct amba_id pl061_ids[] = { { .id = 0x00041061, .mask = 0x000fffff, }, { 0, 0 }, }; MODULE_DEVICE_TABLE(amba, pl061_ids); static struct amba_driver pl061_gpio_driver = { .drv = { .name = "pl061_gpio", #ifdef CONFIG_PM .pm = &pl061_dev_pm_ops, #endif }, .id_table = pl061_ids, .probe = pl061_probe, }; module_amba_driver(pl061_gpio_driver); MODULE_LICENSE("GPL v2"); 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351// SPDX-License-Identifier: GPL-2.0 //! Driver for the ARM PrimeCell(tm) General Purpose Input/Output (PL061). //! //! Based on the C driver written by Baruch Siach <baruch@tkos.co.il>. #![no_std] #![feature(global_asm, allocator_api)] use core::ops::DerefMut; use kernel::{ amba, bit, declare_id_table, device, gpio, io_mem::IoMem, irq::{self, IrqData, LockedIrqData}, power, prelude::*, sync::{IrqDisableSpinLock, Ref}, }; const GPIODIR: usize = 0x400; const GPIOIS: usize = 0x404; const GPIOIBE: usize = 0x408; const GPIOIEV: usize = 0x40C; const GPIOIE: usize = 0x410; const GPIOMIS: usize = 0x418; const GPIOIC: usize = 0x41C; const GPIO_SIZE: usize = 0x1000; const PL061_GPIO_NR: u16 = 8; #[derive(Default)] struct ContextSaveRegs { gpio_data: u8, gpio_dir: u8, gpio_is: u8, gpio_ibe: u8, gpio_iev: u8, gpio_ie: u8, } #[derive(Default)] struct PL061Data { csave_regs: ContextSaveRegs, } struct PL061Resources { base: IoMem<GPIO_SIZE>, parent_irq: u32, } struct PL061Registrations { gpio_chip: gpio::ChipRegistration<PL061Device>, } type DeviceData = device::Data<PL061Registrations, PL061Resources, IrqDisableSpinLock<PL061Data>>; struct PL061Device; impl gpio::Chip for PL061Device { type IrqChip = Self; type Data = Ref<DeviceData>; fn get_direction(data: &Ref<DeviceData>, offset: u32) -> Result<gpio::LineDirection> { let pl061 = data.resources().ok_or(Error::ENXIO)?; Ok(if pl061.base.readb(GPIODIR) & bit(offset) != 0 { gpio::LineDirection::Out } else { gpio::LineDirection::In }) } fn direction_input(data: &Ref<DeviceData>, offset: u32) -> Result { let _guard = data.lock(); let pl061 = data.resources().ok_or(Error::ENXIO)?; let mut gpiodir = pl061.base.readb(GPIODIR); gpiodir &= !bit(offset); pl061.base.writeb(gpiodir, GPIODIR); Ok(()) } fn direction_output(data: &Ref<DeviceData>, offset: u32, value: bool) -> Result { let woffset = bit(offset + 2).into(); let _guard = data.lock(); let pl061 = data.resources().ok_or(Error::ENXIO)?; pl061.base.try_writeb((value as u8) << offset, woffset)?; let mut gpiodir = pl061.base.readb(GPIODIR); gpiodir |= bit(offset); pl061.base.writeb(gpiodir, GPIODIR); // gpio value is set again, because pl061 doesn't allow to set value of a gpio pin before // configuring it in OUT mode. pl061.base.try_writeb((value as u8) << offset, woffset)?; Ok(()) } fn get(data: &Ref<DeviceData>, offset: u32) -> Result<bool> { let pl061 = data.resources().ok_or(Error::ENXIO)?; Ok(pl061.base.try_readb(bit(offset + 2).into())? != 0) } fn set(data: &Ref<DeviceData>, offset: u32, value: bool) { if let Some(pl061) = data.resources() { let woffset = bit(offset + 2).into(); let _ = pl061.base.try_writeb((value as u8) << offset, woffset); } } fn irq_route(data: &Ref<DeviceData>, router: &gpio::IrqRouter) { if let Some(pl061) = data.resources() { let pending = pl061.base.readb(GPIOMIS); if pending != 0 { for offset in 0..PL061_GPIO_NR { if pending & bit(offset) != 0 { router.deliver(offset.into()); } } } } } } impl irq::Chip for PL061Device { type Data = Ref<DeviceData>; fn set_type(data: &Ref<DeviceData>, irq_data: &mut LockedIrqData, trigger: u32) -> Result { let offset = irq_data.hwirq(); let bit = bit(offset); if offset >= PL061_GPIO_NR.into() { return Err(Error::EINVAL); } if trigger & (irq::TYPE_LEVEL_HIGH | irq::TYPE_LEVEL_LOW) != 0 && trigger & (irq::TYPE_EDGE_RISING | irq::TYPE_EDGE_FALLING) != 0 { pr_err!( "trying to configure line {} for both level and edge detection, choose one!\n", offset ); return Err(Error::EINVAL); } let _guard = data.lock(); let pl061 = data.resources().ok_or(Error::ENXIO)?; let mut gpioiev = pl061.base.readb(GPIOIEV); let mut gpiois = pl061.base.readb(GPIOIS); let mut gpioibe = pl061.base.readb(GPIOIBE); if trigger & (irq::TYPE_LEVEL_HIGH | irq::TYPE_LEVEL_LOW) != 0 { let polarity = trigger & irq::TYPE_LEVEL_HIGH != 0; // Disable edge detection. gpioibe &= !bit; // Enable level detection. gpiois |= bit; // Select polarity. if polarity { gpioiev |= bit; } else { gpioiev &= !bit; } irq_data.set_level_handler(); pr_debug!( "line {}: IRQ on {} level\n", offset, if polarity { "HIGH" } else { "LOW" } ); } else if (trigger & irq::TYPE_EDGE_BOTH) == irq::TYPE_EDGE_BOTH { // Disable level detection. gpiois &= !bit; // Select both edges, settings this makes GPIOEV be ignored. gpioibe |= bit; irq_data.set_edge_handler(); pr_debug!("line {}: IRQ on both edges\n", offset); } else if trigger & (irq::TYPE_EDGE_RISING | irq::TYPE_EDGE_FALLING) != 0 { let rising = trigger & irq::TYPE_EDGE_RISING != 0; // Disable level detection. gpiois &= !bit; // Clear detection on both edges. gpioibe &= !bit; // Select edge. if rising { gpioiev |= bit; } else { gpioiev &= !bit; } irq_data.set_edge_handler(); pr_debug!( "line {}: IRQ on {} edge\n", offset, if rising { "RISING" } else { "FALLING}" } ); } else { // No trigger: disable everything. gpiois &= !bit; gpioibe &= !bit; gpioiev &= !bit; irq_data.set_bad_handler(); pr_warn!("no trigger selected for line {}\n", offset); } pl061.base.writeb(gpioiev, GPIOIEV); pl061.base.writeb(gpiois, GPIOIS); pl061.base.writeb(gpioibe, GPIOIBE); Ok(()) } fn mask(data: &Ref<DeviceData>, irq_data: &IrqData) { let mask = bit(irq_data.hwirq() % u64::from(PL061_GPIO_NR)); let _guard = data.lock(); if let Some(pl061) = data.resources() { let gpioie = pl061.base.readb(GPIOIE) & !mask; let _ = pl061.base.try_writeb(gpioie, GPIOIE); } } fn unmask(data: &Ref<DeviceData>, irq_data: &IrqData) { let mask = bit(irq_data.hwirq() % u64::from(PL061_GPIO_NR)); let _guard = data.lock(); if let Some(pl061) = data.resources() { let gpioie = pl061.base.readb(GPIOIE) | mask; let _ = pl061.base.try_writeb(gpioie, GPIOIE); } } // This gets called from the edge IRQ handler to ACK the edge IRQ in the GPIOIC // (interrupt-clear) register. For level IRQs this is not needed: these go away when the level // signal goes away. fn ack(data: &Ref<DeviceData>, irq_data: &IrqData) { let mask = bit(irq_data.hwirq() % u64::from(PL061_GPIO_NR)); let _guard = data.lock(); if let Some(pl061) = data.resources() { let _ = pl061.base.try_writeb(mask.into(), GPIOIC); } } fn set_wake(data: &Ref<DeviceData>, _irq_data: &IrqData, on: bool) -> Result { let pl061 = data.resources().ok_or(Error::ENXIO)?; irq::set_irq_wake(pl061.parent_irq, on) } } impl amba::Driver for PL061Device { type InnerData = DeviceData; type PowerOps = Self; declare_id_table! { (0x00041061, 0x000fffff), } fn probe( dev: &mut amba::Device, _id: &(u32, u32, Option<Self::IdInfo>), ) -> Result<Ref<DeviceData>> { let res = dev.take_resource().ok_or(Error::ENXIO)?; let irq = dev.irq(0).ok_or(Error::ENXIO)?; let data = Ref::try_new_and_init( device::Data::new( PL061Registrations { gpio_chip: gpio::ChipRegistration::default(), }, PL061Resources { base: IoMem::try_new(res)?, parent_irq: irq, }, // SAFETY: We call `irqsave_spinlock_init` below. unsafe { IrqDisableSpinLock::new(PL061Data::default()) }, ), |mut data| { // SAFETY: General part of the data is pinned when `data` is. let gen = unsafe { data.as_mut().map_unchecked_mut(|d| d.deref_mut()) }; kernel::irqdisable_spinlock_init!(gen, "PL061::General"); }, )?; data.resources().ok_or(Error::ENXIO)?.base.writeb(0, GPIOIE); // disable irqs data.registrations() .ok_or(Error::ENXIO)? .gpio_chip .register_with_irq(PL061_GPIO_NR, None, dev, data.clone(), irq)?; pr_info!("PL061 GPIO chip registered\n"); Ok(data) } } impl power::Operations for PL061Device { type Data = Ref<DeviceData>; fn suspend(data: &Ref<DeviceData>) -> Result { let mut inner = data.lock(); let pl061 = data.resources().ok_or(Error::ENXIO)?; inner.csave_regs.gpio_data = 0; inner.csave_regs.gpio_dir = pl061.base.readb(GPIODIR); inner.csave_regs.gpio_is = pl061.base.readb(GPIOIS); inner.csave_regs.gpio_ibe = pl061.base.readb(GPIOIBE); inner.csave_regs.gpio_iev = pl061.base.readb(GPIOIEV); inner.csave_regs.gpio_ie = pl061.base.readb(GPIOIE); for offset in 0..PL061_GPIO_NR { if inner.csave_regs.gpio_dir & bit(offset) != 0 { if let Ok(v) = <Self as gpio::Chip>::get(data, offset.into()) { inner.csave_regs.gpio_data |= (v as u8) << offset; } } } Ok(()) } fn resume(data: &Ref<DeviceData>) -> Result { let inner = data.lock(); let pl061 = data.resources().ok_or(Error::ENXIO)?; for offset in 0..PL061_GPIO_NR { if inner.csave_regs.gpio_dir & bit(offset) != 0 { let value = inner.csave_regs.gpio_data & bit(offset) != 0; let _ = <Self as gpio::Chip>::direction_output(data, offset.into(), value); } else { let _ = <Self as gpio::Chip>::direction_input(data, offset.into()); } } pl061.base.writeb(inner.csave_regs.gpio_is, GPIOIS); pl061.base.writeb(inner.csave_regs.gpio_ibe, GPIOIBE); pl061.base.writeb(inner.csave_regs.gpio_iev, GPIOIEV); pl061.base.writeb(inner.csave_regs.gpio_ie, GPIOIE); Ok(()) } fn freeze(data: &Ref<DeviceData>) -> Result { Self::suspend(data) } fn restore(data: &Ref<DeviceData>) -> Result { Self::resume(data) } } module_amba_driver! { type: PL061Device, name: b"pl061_gpio", author: b"Wedson Almeida Filho", license: b"GPL v2", } (Log in to post comments)

A GPIO driver in Rust Posted Jul 19, 2021 16:15 UTC (Mon) by bredelings (subscriber, #53082) [Link] Nice! Also 75 lines shorter. -BenRI A GPIO driver in Rust Posted Jul 19, 2021 16:18 UTC (Mon) by NHO (subscriber, #104320) [Link] Doesn't actually calls `irqsave_spinlock_init` below A GPIO driver in Rust Posted Jul 19, 2021 17:10 UTC (Mon) by pbonzini (✭ supporter ✭, #60935) [Link] Looks like a typo, there's an irqdisable_spinlock_init below. A GPIO driver in Rust Posted Jul 19, 2021 16:55 UTC (Mon) by logang (subscriber, #127618) [Link] But a lot of the line count difference appears to be due to style... Linux style requires the open brace of the function to be on it's own line, there be a blank line after a function's variable declarations, and often a blank line before the return statement. The Rust code is also using 4 char indents and ignores the line length limit which allows a few lines to require less wrapping. Not to mention the Rust code mixes variable declarations in the body of the code which saves a few more lines and is also against the style guide. If the Rust code didn't ignore the style guide (or the C code was written in a similar compressed style) the line difference would be far less pronounced. IMO all this makes the rust code harder to read than the C code. I suspect the Rust developers would have less resistance if they followed kernel coding style more closely instead of assuming a new language allows them to define their own style. The biggest line savings seem to be in the include list (where rust can put multiple includes on a single line) and the initialization of the gpiochip and irqchip methods (which Rust gets away without needing by adding an additional indent on the definition of those methods). A GPIO driver in Rust Posted Jul 19, 2021 18:04 UTC (Mon) by mathstuf (subscriber, #69389) [Link] > there be a blank line after a function's variable declarations In Rust, one can do `let` declarations at the top of the function, but I find `let result = { /* block which computes */ };` instead of `int ret; /* decls and setup code */; ret = computed_result;` and not have `ret` be `const` is *far* worse. Rust can do the latter *and* have it be `const`, but…why do the hoist? > Not to mention the Rust code mixes variable declarations in the body of the code which saves a few more lines and is also against the style guide. In Rust, this can be *very* important if the lifetime of a variable is not available until the middle of the function. Declaring it at the top of the function can make its lifetime "too long" to pass off to some narrower lifetime API. > I suspect the Rust developers would have less resistance if they followed kernel coding style more closely instead of assuming a new language allows them to define their own style. Some of the rules come from C being C. Those are fine to let lie on the floor. Determining which is which however is where the flamewars then lie… Variable declarations needing to be hoisted (and the "Christmas tree" correlary) are such rules. Indentation, extra newlines, and brace placement? I like hoisted braces for vertical savings (which allows for more spacing between "sections" of code too. Indentation is something I've come to care less about as long as it's "enough" to be able to find a matching section by eyeball. 2 is…fine, but not ideal. 4 is great. 8 is OK (though I find people's consistency for using spaces-for-alignment in tabulator-using source lacking). Tabs also break alignment in diffs (my main pet peeve with them). A GPIO driver in Rust Posted Jul 19, 2021 19:06 UTC (Mon) by jezuch (subscriber, #52988) [Link] > If the Rust code didn't ignore the style guide (or the C code was written in a similar compressed style) Rust is not ignoring the style guide, it's just using a different one (Rust's). In contrast to C and almost all languages before it, Rust has an official style guide. You deviate from it at your own peril! A GPIO driver in Rust Posted Jul 19, 2021 19:15 UTC (Mon) by kay (subscriber, #1362) [Link] > But a lot of the line count difference appears to be due to style... [explanation why kernel style has more lines] OTH the rust code always use braces for if and else statements, the C code does not in case of a single instruction if or else. Nevertheless it's not the line count that counts ;-) A GPIO driver in Rust Posted Jul 19, 2021 17:07 UTC (Mon) by jgg (subscriber, #55211) [Link] Is all the type erasure a good idea? ... data: &Ref<DeviceData> ... let _guard = data.lock(); Quick! Tell me if this code is now in an atomic context? Should it use GFP_KERNEL or GFP_ATOMIC? Does rust check this kind of stuff at compile time? This is not an improvement (again, what type is this, what do the values mean?): declare_id_table! { (0x00041061, 0x000fffff), } vs static const struct amba_id pl061_ids[] = { { .id = 0x00041061, .mask = 0x000fffff, }, The CONFIG_PM memory consumption optimization seems to have been lost? The trick with the drvdata is... interesting.. I wonder how that works with something like sysfs files that often will read the drvdata? The ordering of the set will be wrong. Could rust tell at compile time? A GPIO driver in Rust Posted Jul 19, 2021 17:16 UTC (Mon) by pbonzini (✭ supporter ✭, #60935) [Link] > Quick! Tell me if this code is now in an atomic context? Should it use GFP_KERNEL or GFP_ATOMIC? You trade that for not having to remember, for every spinlock, whether it's the irqsave kind or the "normal" one. It's probably possible to improve on both the Rust problem (too much type erasure) and the C problem. > Does rust check this kind of stuff at compile time? In some cases, it's probably possible to pass in a guard that ensures that a given function is called with disabled IRQ or with a given spinlock taken. A GPIO driver in Rust Posted Jul 19, 2021 18:36 UTC (Mon) by jgg (subscriber, #55211) [Link] The use of spinlock irqsave/irq/bh/etc is very situational depending on the design of the system. The irqsave version is supposed to be irqsave in normal processes, but just spin_lock() in the IRQ (and thus should have an acquire on an IRQ path). Further if you know you are not already in an irq disabled region you should be using just spin_lock_irq() Collapsing everything to a irqsave is a simplification, but not necessarily an improvement.. A GPIO driver in Rust Posted Jul 19, 2021 17:45 UTC (Mon) by farnz (subscriber, #17727) [Link] > declare_id_table! { > (0x00041061, 0x000fffff), > } This declares a table of IDs (hence the macro name); it would be a simple change to make it: declare_amba_id_table! { (id = 0x00041061, mask = 0x000fffff), } if the Rust binding authors want you to, so that it's (a) clear this is an AMBA ID table, and (b) the id and mask elements are clearly demarcated. That's a stylistic choice that Linus et al can definitely weigh in on. A GPIO driver in Rust Posted Jul 19, 2021 17:08 UTC (Mon) by rvolgers (subscriber, #63218) [Link] It's funny, seeing them side by side like this seems to activate the "reading C" part of my brain while looking at the Rust code. I notice I get annoyed at the lack of explicit types on variables and things like ".into()" (which likewise doesn't offer much clue as to typing, just that a conversion is happening). In C, it's relatively important to be aware of types. On one hand due to various traps related to UB and integer arithmetic, and on the other hand because there are no guard rails on most APIs. Rust really encourages you to embed restrictions in the API, so that the use site can be a lot more terse. Also with "C brain" engaged, some of the nested control flow like in `Chip::get_direction` seems weird. But objectively, I think the Rust version is quicker to read. I do notice that the indentation is 4 spaces in the Rust version. Pretty sure that's because they're using Rust defaults for formatting and also pretty sure there has been a lot of discussion about this already and I really don't want to start that again. But just mentioning it since it does make nesting look a little worse in the C case. Machine code Posted Jul 19, 2021 19:52 UTC (Mon) by ikm (subscriber, #493) [Link] Did anyone try comparing the resulting machine code for both versions?

Leave a Comment