Messages in this thread | | | Subject | Re: [PATCH] [net/irda]: new Blackfin on-chip SIR IrDA driver | From | gyang <> | Date | Thu, 12 Mar 2009 12:17:35 +0800 |
| |
On Wed, 2009-03-11 at 06:43 -0400, Mike Frysinger wrote: > On Wed, Mar 11, 2009 at 05:56, graff yang wrote: > > On Wed, Mar 11, 2009 at 3:57 PM, Mike Frysinger wrote: > >> On Wed, Mar 11, 2009 at 03:29, <graff.yang@gmail.com> wrote: > >>> +static int __devinit bfin_sir_probe(struct platform_device *pdev) > >>> +{ > >>> + struct net_device *dev; > >>> + struct bfin_sir_self *self; > >>> + unsigned int baudrate_mask; > >>> + struct bfin_sir_port *sir_port; > >>> + int err; > >>> + > >>> + err = peripheral_request_list(per[pdev->id], DRIVER_NAME); > >> > >> what if pdev->id is set to 12512 ? > > > > The pdev->id is defined in board files, for example, uart0 should be 0. > > so ? what's to stop the user from setting it to 12415 ? the driver > must sanity check these things. you can never assume platform dev > resources always have valid pointers and valid values.
How about to limit the pdev->id to be 0,1,2,3 ? I can extend the per to be static const unsigned short per[][4] = { {P_UART0_RX, P_UART0_TX, 0, 0}, {P_UART1_RX, P_UART1_TX, 0, 1}, {P_UART2_RX, P_UART2_TX, 0, 2}, {P_UART3_RX, P_UART3_TX, 0, 3}, };
then check (pdev->id >= 0 && pdev->id < ARRAY_SIZE(per) && per[pdev->id][3] == pdev->id)
> > >>> + switch (max_rate) { > >>> + default: > >>> + printk(KERN_WARNING "bfin_sir: Invalid maximum baud rate, using 9600\n"); > >> > >> dev_warn(&dev->dev,....) ? > > > > Here the netdev has not been registered. > > then use pdev->dev ? Don't it seem strange to mixed use dev_warn(&dev->dev, ...) and dev_warn(&pdev->dev, ...) ? And printk is enough.
> -mike
| |