linux-kernel - Re: [Patch 2/3] via-sdmmc: via-sdmmc.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 16 Dec 2008 16:00:12 +0000
From:	Ben Dooks <ben-linux@...ff.org>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	JosephChan@....com.tw, linux-kernel@...r.kernel.org
Subject: Re: [Patch 2/3] via-sdmmc: via-sdmmc.c

On Tue, Dec 16, 2008 at 02:31:42PM +0100, Arnd Bergmann wrote:
> On Tuesday 16 December 2008, JosephChan@....com.tw wrote:
> > VIA MSP SD/MMC card reader driver of via-sdmmc
> 
> The code looks very nice, good work.
> 
> > +static void via_save_pcictrlreg(struct via_crdr_chip *vcrdr_chip)
> > +{
> > +	struct pcictrlreg *pm_pcictrl_reg;
> > +	void __iomem *addrbase;
> > +
> > +	pm_pcictrl_reg = &(vcrdr_chip->pm_pcictrl_reg);
> > +	addrbase = vcrdr_chip->pcictrl_mmiobase;
> > +
> > +	pm_pcictrl_reg->pciclkgat_reg = readb(addrbase + PCICLKGATT_REG);
> > +	pm_pcictrl_reg->pciclkgat_reg |=
> > +	    PCI_CLKGATT_POWSEL | PCI_CLKGATT_POWOFF;
> > +	pm_pcictrl_reg->pcimscclk_reg = readb(addrbase + PCIMSCCLK_REG);
> > +	pm_pcictrl_reg->pcisdclk_reg = readb(addrbase + PCISDCCLK_REG);
> > +	pm_pcictrl_reg->pcidmaclk_reg = readb(addrbase + PCIDMACLK_REG);
> > +	pm_pcictrl_reg->pciintctrl_reg = readb(addrbase + PCIINTCTRL_REG);
> > +	pm_pcictrl_reg->pciintstatus_reg = readb(addrbase + PCIINTSTATUS_REG);
> > +	pm_pcictrl_reg->pcitmoctrl_reg = readb(addrbase + PCITMOCTRL_REG);
> > +}
> 
> Since you already define the data structure for the save area, how about
> using it for the register accesses as well? You could drop all the PCI*_REG
> macro definitions and do

GAH! NO, I belive Linus has very clear views about using structures
to define register layouts (and I share them).
 
> struct pcictrlreg __iomem *pcr = vcrdr_chip->pcictrl_mmiobase;
> pm_pcictrl_reg->pcisdclk_reg = readb(&pcr->pcisdclk_reg);

-- 
Ben (ben@...ff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/