diff for duplicates of <5538C6C2.5010508@linaro.org> diff --git a/a/1.txt b/N1/1.txt index 2e24864..d2a8ae1 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -26,7 +26,7 @@ On 04/23/2015 03:00 AM, Nishanth Menon wrote: >> "Also, it is recommended that the alarm registers be loaded >> before the alarm is enabled." >> -> +>=20 > Hmm... i did not know that existed, thanks for digging it up.. that > teaches me to look for docs before putting a scope/LA on the board > (not that I regret doing that)... That said, reading the app note, I @@ -35,22 +35,22 @@ On 04/23/2015 03:00 AM, Nishanth Menon wrote: > then, that implies that oscillator will have to be restarted (upto a > few seconds for certain crystals).. but that said, it does not seem > mandatory or seem to (yet seen) functional issues... -> +>=20 > b) We dont have flexibility yet to describe if we do indeed have a > backup battery or not - VBATEN should be set only if we have a backup > battery on the platform :( - on some it might even be optional thanks > to certain compliance requirements of shipping boards internationally > and general "unlike" of lithium ion in cargo hold.. -> +>=20 > c) we dont have capability to control the alarm polarity in the driver > which, by the way, we probably should also control OUT polarity (when > ALARM is not asserted).. -> +>=20 > d) we dont have support for external 32k oscillator(X1 only) instead > of assuming we always have a 32k crystal(X1 and X2)... -> +>=20 > Ugghhh... more cleaning up to do for the future.. -> +>=20 > that said, the sequence it does recommend (in page 4): > The following steps show how the Alarm 0 is config- > ured. Alarm 1 can be configured in the same manner. @@ -59,47 +59,47 @@ On 04/23/2015 03:00 AM, Nishanth Menon wrote: > 2. Write 0x47 to the Alarm0 Minutes register > [0x0B]. > 3. Write 0x71 to the Alarm0 Hours register [0x0C] -> – 11 hours in 12-hour format. -> 4. Write 0x72 to the Alarm0 Day register [0x0D] – +> =E2=80=93 11 hours in 12-hour format. +> 4. Write 0x72 to the Alarm0 Day register [0x0D] =E2=80=93 > Tuesday + Alarm Polarity Low + Match on all. > The Alarm0 Interrupt Flag is also cleared. > 5. Write 0x14 to the Alarm0 Date register [0x0E]. > 6. Write 0x08 to the Alarm0 Month register [0x0F]. > With all the Alarm0 registers set we can now activate > the Alarm0 on the Control register. -> 7. Write 0x10 to the Control register [0x07] – +> 7. Write 0x10 to the Control register [0x07] =E2=80=93 > Alarm0 enabled no CLKOUT, Alarm1 disabled -> +>=20 > before this patch we do ( http://pastebin.ubuntu.com/10863880/) -> CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) -> OSCTRIM r[8] = 0x00 -> EEUNLOCK r[9] = 0x00 -> ALM0SEC r[A] = 0x01 -> ALM0MIN r[B] = 0x45 -> ALM0HOUR r[C] = 0x23 -> ALM0WKDAY r[D] = 0x75 <-ALMOIF is cleared -> ALM0DATE r[E] = 0x09 -> ALM0MTH r[F] = 0x04 -> RSRVED r[10] = 0x01 -> +> CONTROL r[7] =3D 0x90 (OUT=3D1, ALM0EN=3D1) +> OSCTRIM r[8] =3D 0x00 +> EEUNLOCK r[9] =3D 0x00 +> ALM0SEC r[A] =3D 0x01 +> ALM0MIN r[B] =3D 0x45 +> ALM0HOUR r[C] =3D 0x23 +> ALM0WKDAY r[D] =3D 0x75 <-ALMOIF is cleared +> ALM0DATE r[E] =3D 0x09 +> ALM0MTH r[F] =3D 0x04 +> RSRVED r[10] =3D 0x01 +>=20 > with this patch, we do: -> burst( CONTROL r[7] = 0x80 (OUT=1) -> OSCTRIM r[8] = 0x00 -> EEUNLOCK r[9] = 0x00 -> ALM0SEC r[A] = 0x01 -> ALM0MIN r[B] = 0x45 -> ALM0HOUR r[C] = 0x23 -> ALM0WKDAY r[D] = 0x75 <-ALMOIF is cleared -> ALM0DATE r[E] = 0x09 -> ALM0MTH r[F] = 0x04 -> RSRVED r[10] = 0x01 +> burst( CONTROL r[7] =3D 0x80 (OUT=3D1) +> OSCTRIM r[8] =3D 0x00 +> EEUNLOCK r[9] =3D 0x00 +> ALM0SEC r[A] =3D 0x01 +> ALM0MIN r[B] =3D 0x45 +> ALM0HOUR r[C] =3D 0x23 +> ALM0WKDAY r[D] =3D 0x75 <-ALMOIF is cleared +> ALM0DATE r[E] =3D 0x09 +> ALM0MTH r[F] =3D 0x04 +> RSRVED r[10] =3D 0x01 > ) -> CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) -> +> CONTROL r[7] =3D 0x90 (OUT=3D1, ALM0EN=3D1) +>=20 > Which is slightly unoptimal way of what the app note recommends. - as > I mentioned earlier in this thread, I will try and do optimizations in > a later patch. -> +>=20 > Given that Andrew had picked up this patch, I dont see a reason to > respin this yet. but will include the app note for future patches - > thanks for pointing it out to me. @@ -122,27 +122,31 @@ On 04/23/2015 03:00 AM, Nishanth Menon wrote: >>> index 4ffabb322a9a..3cd4783375a5 100644 >>> --- a/drivers/rtc/rtc-ds1307.c >>> +++ b/drivers/rtc/rtc-ds1307.c ->>> @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t) ->>> regs[6] &= ~MCP794XX_BIT_ALMX_IF; +>>> @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev,= + struct rtc_wkalrm *t) +>>> regs[6] &=3D ~MCP794XX_BIT_ALMX_IF; >>> /* Set alarm match: second, minute, hour, day, date, month. */ ->>> regs[6] |= MCP794XX_MSK_ALMX_MATCH; +>>> regs[6] |=3D MCP794XX_MSK_ALMX_MATCH; >>> - >>> - if (t->enabled) ->>> - regs[0] |= MCP794XX_BIT_ALM0_EN; +>>> - regs[0] |=3D MCP794XX_BIT_ALM0_EN; >>> - else ->>> - regs[0] &= ~MCP794XX_BIT_ALM0_EN; ->>> + /* Disable interrupt. We will not enable until completely programmed */ ->>> + regs[0] &= ~MCP794XX_BIT_ALM0_EN; ->>> ->>> ret = ds1307->write_block_data(client, MCP794XX_REG_CONTROL, 10, regs); +>>> - regs[0] &=3D ~MCP794XX_BIT_ALM0_EN; +>>> + /* Disable interrupt. We will not enable until completely programmed = +*/ +>>> + regs[0] &=3D ~MCP794XX_BIT_ALM0_EN; +>>> =20 +>>> ret =3D ds1307->write_block_data(client, MCP794XX_REG_CONTROL, 10, = +regs); >>> if (ret < 0) >>> return ret; ->>> +>>> =20 >>> - return 0; >>> + if (!t->enabled) >>> + return 0; ->>> + regs[0] |= MCP794XX_BIT_ALM0_EN; ->>> + return i2c_smbus_write_byte_data(client, MCP794XX_REG_CONTROL, regs[0]); +>>> + regs[0] |=3D MCP794XX_BIT_ALM0_EN; +>>> + return i2c_smbus_write_byte_data(client, MCP794XX_REG_CONTROL, regs[0= +]); >> >> So, It seems, that right sequence should be: >> - disable alarmX @@ -155,25 +159,37 @@ On 04/23/2015 03:00 AM, Nishanth Menon wrote: > performed. probably just a couple of reads might be > sufficient..(ALM0WKDAY has some control bits as well.. Ugggh.. > anyways..)... -> -> +>=20 +>=20 > Will have to think more about optimizing more later. -Also I've done some fast investigation and I found that ~half of RTC drivers disable - ALM IRQ before start accessing Alarm regs (twl-rtc.c) while another half don't do that :) +Also I've done some fast investigation and I found that ~half of RTC driver= +s disable + ALM IRQ before start accessing Alarm regs (twl-rtc.c) while another half = +don't do that :) (just FYI) -> +>=20 >> ->> More over, looks like, alarm/alarm IRQ should be enabled/disabled separately from set_alarm/RTC_ALM_SET +>> More over, looks like, alarm/alarm IRQ should be enabled/disabled separa= +tely from set_alarm/RTC_ALM_SET >> by RTC_AIE_ON, RTC_AIE_OFF. Should it? -> +>=20 --- +--=20 regards, -grygorii --- -To unsubscribe from this list: send the line "unsubscribe linux-omap" in -the body of a message to majordomo@vger.kernel.org -More majordomo info at http://vger.kernel.org/majordomo-info.html + +--=20 +--=20 +You received this message because you are subscribed to "rtc-linux". +Membership options at http://groups.google.com/group/rtc-linux . +Please read http://groups.google.com/group/rtc-linux/web/checklist +before submitting a driver. +---=20 +You received this message because you are subscribed to the Google Groups "= +rtc-linux" group. +To unsubscribe from this group and stop receiving emails from it, send an e= +mail to rtc-linux+unsubscribe@googlegroups.com. +For more options, visit https://groups.google.com/d/optout. diff --git a/a/content_digest b/N1/content_digest index dda4b2e..7335413 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,7 +2,7 @@ "ref\05537A169.206@linaro.org\0" "ref\055383625.3070108@ti.com\0" "From\0Grygorii.Strashko@linaro.org <grygorii.strashko@linaro.org>\0" - "Subject\0Re: [PATCH V2] drivers/rtc/rtc-ds1307.c: Enable the mcp794xx alarm after programming time\0" + "Subject\0[rtc-linux] Re: [PATCH V2] drivers/rtc/rtc-ds1307.c: Enable the mcp794xx alarm after programming time\0" "Date\0Thu, 23 Apr 2015 13:17:38 +0300\0" "To\0Nishanth Menon <nm@ti.com>" Alexandre Belloni <alexandre.belloni@free-electrons.com> @@ -40,7 +40,7 @@ ">> \"Also, it is recommended that the alarm registers be loaded\n" ">> before the alarm is enabled.\"\n" ">>\n" - "> \n" + ">=20\n" "> Hmm... i did not know that existed, thanks for digging it up.. that\n" "> teaches me to look for docs before putting a scope/LA on the board\n" "> (not that I regret doing that)... That said, reading the app note, I\n" @@ -49,22 +49,22 @@ "> then, that implies that oscillator will have to be restarted (upto a\n" "> few seconds for certain crystals).. but that said, it does not seem\n" "> mandatory or seem to (yet seen) functional issues...\n" - "> \n" + ">=20\n" "> b) We dont have flexibility yet to describe if we do indeed have a\n" "> backup battery or not - VBATEN should be set only if we have a backup\n" "> battery on the platform :( - on some it might even be optional thanks\n" "> to certain compliance requirements of shipping boards internationally\n" "> and general \"unlike\" of lithium ion in cargo hold..\n" - "> \n" + ">=20\n" "> c) we dont have capability to control the alarm polarity in the driver\n" "> which, by the way, we probably should also control OUT polarity (when\n" "> ALARM is not asserted)..\n" - "> \n" + ">=20\n" "> d) we dont have support for external 32k oscillator(X1 only) instead\n" "> of assuming we always have a 32k crystal(X1 and X2)...\n" - "> \n" + ">=20\n" "> Ugghhh... more cleaning up to do for the future..\n" - "> \n" + ">=20\n" "> that said, the sequence it does recommend (in page 4):\n" "> The following steps show how the Alarm 0 is config-\n" "> ured. Alarm 1 can be configured in the same manner.\n" @@ -73,47 +73,47 @@ "> 2. Write 0x47 to the Alarm0 Minutes register\n" "> [0x0B].\n" "> 3. Write 0x71 to the Alarm0 Hours register [0x0C]\n" - "> \342\200\223 11 hours in 12-hour format.\n" - "> 4. Write 0x72 to the Alarm0 Day register [0x0D] \342\200\223\n" + "> =E2=80=93 11 hours in 12-hour format.\n" + "> 4. Write 0x72 to the Alarm0 Day register [0x0D] =E2=80=93\n" "> Tuesday + Alarm Polarity Low + Match on all.\n" "> The Alarm0 Interrupt Flag is also cleared.\n" "> 5. Write 0x14 to the Alarm0 Date register [0x0E].\n" "> 6. Write 0x08 to the Alarm0 Month register [0x0F].\n" "> With all the Alarm0 registers set we can now activate\n" "> the Alarm0 on the Control register.\n" - "> 7. Write 0x10 to the Control register [0x07] \342\200\223\n" + "> 7. Write 0x10 to the Control register [0x07] =E2=80=93\n" "> Alarm0 enabled no CLKOUT, Alarm1 disabled\n" - "> \n" + ">=20\n" "> before this patch we do ( http://pastebin.ubuntu.com/10863880/)\n" - "> \tCONTROL r[7] = 0x90 (OUT=1, ALM0EN=1)\n" - "> \tOSCTRIM r[8] = 0x00\n" - "> \tEEUNLOCK r[9] = 0x00\n" - "> \tALM0SEC r[A] = 0x01\n" - "> \tALM0MIN r[B] = 0x45\n" - "> \tALM0HOUR r[C] = 0x23\n" - "> \tALM0WKDAY r[D] = 0x75 <-ALMOIF is cleared\n" - "> \tALM0DATE r[E] = 0x09\n" - "> \tALM0MTH r[F] = 0x04\n" - "> \tRSRVED r[10] = 0x01\n" - "> \n" + "> \tCONTROL r[7] =3D 0x90 (OUT=3D1, ALM0EN=3D1)\n" + "> \tOSCTRIM r[8] =3D 0x00\n" + "> \tEEUNLOCK r[9] =3D 0x00\n" + "> \tALM0SEC r[A] =3D 0x01\n" + "> \tALM0MIN r[B] =3D 0x45\n" + "> \tALM0HOUR r[C] =3D 0x23\n" + "> \tALM0WKDAY r[D] =3D 0x75 <-ALMOIF is cleared\n" + "> \tALM0DATE r[E] =3D 0x09\n" + "> \tALM0MTH r[F] =3D 0x04\n" + "> \tRSRVED r[10] =3D 0x01\n" + ">=20\n" "> with this patch, we do:\n" - "> burst(\tCONTROL r[7] = 0x80 (OUT=1)\n" - "> \tOSCTRIM r[8] = 0x00\n" - "> \tEEUNLOCK r[9] = 0x00\n" - "> \tALM0SEC r[A] = 0x01\n" - "> \tALM0MIN r[B] = 0x45\n" - "> \tALM0HOUR r[C] = 0x23\n" - "> \tALM0WKDAY r[D] = 0x75 <-ALMOIF is cleared\n" - "> \tALM0DATE r[E] = 0x09\n" - "> \tALM0MTH r[F] = 0x04\n" - "> \tRSRVED r[10] = 0x01\n" + "> burst(\tCONTROL r[7] =3D 0x80 (OUT=3D1)\n" + "> \tOSCTRIM r[8] =3D 0x00\n" + "> \tEEUNLOCK r[9] =3D 0x00\n" + "> \tALM0SEC r[A] =3D 0x01\n" + "> \tALM0MIN r[B] =3D 0x45\n" + "> \tALM0HOUR r[C] =3D 0x23\n" + "> \tALM0WKDAY r[D] =3D 0x75 <-ALMOIF is cleared\n" + "> \tALM0DATE r[E] =3D 0x09\n" + "> \tALM0MTH r[F] =3D 0x04\n" + "> \tRSRVED r[10] =3D 0x01\n" "> )\n" - "> \tCONTROL r[7] = 0x90 (OUT=1, ALM0EN=1)\n" - "> \n" + "> \tCONTROL r[7] =3D 0x90 (OUT=3D1, ALM0EN=3D1)\n" + ">=20\n" "> Which is slightly unoptimal way of what the app note recommends. - as\n" "> I mentioned earlier in this thread, I will try and do optimizations in\n" "> a later patch.\n" - "> \n" + ">=20\n" "> Given that Andrew had picked up this patch, I dont see a reason to\n" "> respin this yet. but will include the app note for future patches -\n" "> thanks for pointing it out to me.\n" @@ -136,27 +136,31 @@ ">>> index 4ffabb322a9a..3cd4783375a5 100644\n" ">>> --- a/drivers/rtc/rtc-ds1307.c\n" ">>> +++ b/drivers/rtc/rtc-ds1307.c\n" - ">>> @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)\n" - ">>> \tregs[6] &= ~MCP794XX_BIT_ALMX_IF;\n" + ">>> @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev,=\n" + " struct rtc_wkalrm *t)\n" + ">>> \tregs[6] &=3D ~MCP794XX_BIT_ALMX_IF;\n" ">>> \t/* Set alarm match: second, minute, hour, day, date, month. */\n" - ">>> \tregs[6] |= MCP794XX_MSK_ALMX_MATCH;\n" + ">>> \tregs[6] |=3D MCP794XX_MSK_ALMX_MATCH;\n" ">>> -\n" ">>> -\tif (t->enabled)\n" - ">>> -\t\tregs[0] |= MCP794XX_BIT_ALM0_EN;\n" + ">>> -\t\tregs[0] |=3D MCP794XX_BIT_ALM0_EN;\n" ">>> -\telse\n" - ">>> -\t\tregs[0] &= ~MCP794XX_BIT_ALM0_EN;\n" - ">>> +\t/* Disable interrupt. We will not enable until completely programmed */\n" - ">>> +\tregs[0] &= ~MCP794XX_BIT_ALM0_EN;\n" - ">>> \n" - ">>> \tret = ds1307->write_block_data(client, MCP794XX_REG_CONTROL, 10, regs);\n" + ">>> -\t\tregs[0] &=3D ~MCP794XX_BIT_ALM0_EN;\n" + ">>> +\t/* Disable interrupt. We will not enable until completely programmed =\n" + "*/\n" + ">>> +\tregs[0] &=3D ~MCP794XX_BIT_ALM0_EN;\n" + ">>> =20\n" + ">>> \tret =3D ds1307->write_block_data(client, MCP794XX_REG_CONTROL, 10, =\n" + "regs);\n" ">>> \tif (ret < 0)\n" ">>> \t\treturn ret;\n" - ">>> \n" + ">>> =20\n" ">>> -\treturn 0;\n" ">>> +\tif (!t->enabled)\n" ">>> +\t\treturn 0;\n" - ">>> +\tregs[0] |= MCP794XX_BIT_ALM0_EN;\n" - ">>> +\treturn i2c_smbus_write_byte_data(client, MCP794XX_REG_CONTROL, regs[0]);\n" + ">>> +\tregs[0] |=3D MCP794XX_BIT_ALM0_EN;\n" + ">>> +\treturn i2c_smbus_write_byte_data(client, MCP794XX_REG_CONTROL, regs[0=\n" + "]);\n" ">>\n" ">> So, It seems, that right sequence should be:\n" ">> - disable alarmX\n" @@ -169,27 +173,39 @@ "> performed. probably just a couple of reads might be\n" "> sufficient..(ALM0WKDAY has some control bits as well.. Ugggh..\n" "> anyways..)...\n" - "> \n" - "> \n" + ">=20\n" + ">=20\n" "> Will have to think more about optimizing more later.\n" "\n" "\n" - "Also I've done some fast investigation and I found that ~half of RTC drivers disable\n" - " ALM IRQ before start accessing Alarm regs (twl-rtc.c) while another half don't do that :)\n" + "Also I've done some fast investigation and I found that ~half of RTC driver=\n" + "s disable\n" + " ALM IRQ before start accessing Alarm regs (twl-rtc.c) while another half =\n" + "don't do that :)\n" "(just FYI)\n" "\n" - "> \n" + ">=20\n" ">>\n" - ">> More over, looks like, alarm/alarm IRQ should be enabled/disabled separately from set_alarm/RTC_ALM_SET\n" + ">> More over, looks like, alarm/alarm IRQ should be enabled/disabled separa=\n" + "tely from set_alarm/RTC_ALM_SET\n" ">> by RTC_AIE_ON, RTC_AIE_OFF. Should it?\n" - "> \n" + ">=20\n" "\n" - "-- \n" + "--=20\n" "regards,\n" "-grygorii\n" - "--\n" - "To unsubscribe from this list: send the line \"unsubscribe linux-omap\" in\n" - "the body of a message to majordomo@vger.kernel.org\n" - More majordomo info at http://vger.kernel.org/majordomo-info.html + "\n" + "--=20\n" + "--=20\n" + "You received this message because you are subscribed to \"rtc-linux\".\n" + "Membership options at http://groups.google.com/group/rtc-linux .\n" + "Please read http://groups.google.com/group/rtc-linux/web/checklist\n" + "before submitting a driver.\n" + "---=20\n" + "You received this message because you are subscribed to the Google Groups \"=\n" + "rtc-linux\" group.\n" + "To unsubscribe from this group and stop receiving emails from it, send an e=\n" + "mail to rtc-linux+unsubscribe@googlegroups.com.\n" + For more options, visit https://groups.google.com/d/optout. -0721a2d955f49d57b1571f74bd2be9615ba788b3f3f110d723f530ebd05a5a9c +04cd42180dbe488f34b8520efe44f2d892a0527a2f036333e80caec1169f1a93
diff --git a/a/1.txt b/N2/1.txt index 2e24864..81d9f08 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -173,7 +173,3 @@ Also I've done some fast investigation and I found that ~half of RTC drivers dis -- regards, -grygorii --- -To unsubscribe from this list: send the line "unsubscribe linux-omap" in -the body of a message to majordomo@vger.kernel.org -More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N2/content_digest index dda4b2e..16465fd 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -186,10 +186,6 @@ "\n" "-- \n" "regards,\n" - "-grygorii\n" - "--\n" - "To unsubscribe from this list: send the line \"unsubscribe linux-omap\" in\n" - "the body of a message to majordomo@vger.kernel.org\n" - More majordomo info at http://vger.kernel.org/majordomo-info.html + -grygorii -0721a2d955f49d57b1571f74bd2be9615ba788b3f3f110d723f530ebd05a5a9c +58019fc62ea6c5b33b2623c7db1baf45404b3fc4cb0f07ab6a3ce768c8bfab78
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.