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]
Message-ID: <20081216160012.GE12431@fluff.org.uk>
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ