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
| ||
|
Date: Fri, 29 Aug 2014 10:41:14 +0300 From: Matti Vaittinen <matti.vaittinen@....com> To: ext Mark Rutland <mark.rutland@....com> Cc: "a.zummo@...ertech.it" <a.zummo@...ertech.it>, "jason@...edaemon.net" <jason@...edaemon.net>, "linux@...ck-us.net" <linux@...ck-us.net>, "jic23@...nel.org" <jic23@...nel.org>, "arno@...isbad.org" <arno@...isbad.org>, "jgunthorpe@...idianresearch.com" <jgunthorpe@...idianresearch.com>, "san@...etechnology.dk" <san@...etechnology.dk>, "hs@...x.de" <hs@...x.de>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "rtc-linux@...glegroups.com" <rtc-linux@...glegroups.com>, Sverdlin Alexander <alexander.sverdlin@....com> Subject: Re: [PATCH] rtc: ds1307: add trickle charger device tree binding On Thu, Aug 28, 2014 at 01:59:15PM +0100, ext Mark Rutland wrote: > On Thu, Aug 28, 2014 at 01:42:44PM +0100, Matti Vaittinen wrote: > > Patch adding support for specifying trickle charger setup from device > > tree. Patch is based on linux-next tree. > > > > > > Some DS13XX devices have "trickle chargers". Introduce a device tree binding > > for specifying the setup and register values. > > > > diff --git a/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt b/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt > > new file mode 100644 > > index 0000000..9faf40e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/rtc/dallas,ds1339.txt > > @@ -0,0 +1,19 @@ > > +* Dallas DS1339 I2C Serial Real-Time Clock > > + > > +Required properties: > > +- compatible: Should contain "dallas,ds1339". > > +- reg: I2C address for chip > > + > > +Optional properties: > > +- trickle_setup : Used Trickle Charger configuration, > > + corresponding to 5 lowest bits in trickle charger register. > > s/_/-/ in property names please. I'll fix this, thanks. > Why do we not have a higher-level description? I can add the meanings for this specific chip in the binding document, however I would still keep the raw register value as the dt entry due to reasons explained in my previous email. > > > +- trickle_reg : Trickle charger register address. Defaults to 0x10 for ds1339. > > Is this expected to change? Why does this need to be in the DT? I was thinking it might be handly as I do not know the details of all variants of this chip. However I guess it could be left out. > > > + > > +Example: > > + ds1339: rtc@68 { > > + compatible = "dallas,ds1339"; > > + trickle_setup= <0x5>; > > + reg = <0x68>; > > + }; > > + > > + > > diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c > > index f03d5ba..f9b1244 100644 > > --- a/drivers/rtc/rtc-ds1307.c > > +++ b/drivers/rtc/rtc-ds1307.c > > @@ -126,9 +126,10 @@ struct chip_desc { > > u16 nvram_offset; > > u16 nvram_size; > > u16 trickle_charger_reg; > > + u8 trickle_charger_setup; > > }; > > > > -static const struct chip_desc chips[last_ds_type] = { > > +static struct chip_desc chips[last_ds_type] = { > > [ds_1307] = { > > .nvram_offset = 8, > > .nvram_size = 56, > > @@ -835,13 +836,37 @@ ds1307_nvram_write(struct file *filp, struct kobject *kobj, > > > > /*----------------------------------------------------------------------*/ > > > > +static void trickle_charger_of_init(struct chip_desc *chip, > > + struct device_node *node) > > +{ > > + int plen = 0; > > + const uint32_t *setup; > > + const uint32_t *reg; > > + > > + setup = of_get_property(node, "trickle_setup" , &plen); > > + if (plen != 4) > > + goto out; > > + chip->trickle_charger_setup = (u8)*setup; > > This isn't endian-clean, and the casting is unnecessary. > > Use the of_property_read_u32 accessor, with a u32 temporary variable. I'll fix this, thanks. > > > + plen = 0; > > + reg = of_get_property(node, "trickle_reg" , &plen); > > + if (plen == 4) > > + chip->trickle_charger_reg = (u16)*reg; > > Likewise. > I'll drop the register address from dt alltogether. I will fix these issues and prepare a new patch. Br. Matti Vaittinen -- ============================================= Matti Vaittinen Senile SW Specialist FINLAND ~~ When things go utterly wrong vim users can always type :help! ~~ -- 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