* Re: Looking for issues/features for my first contribution
2019-11-08 2:39 ` Rajath Shashidhara
@ 2019-11-08 9:08 ` Alex Bennée
2019-11-08 13:05 ` Richard Henderson
2019-11-08 19:31 ` Aleksandar Markovic
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2019-11-08 9:08 UTC (permalink / raw)
To: qemu-devel; +Cc: Aleksandar Markovic
Rajath Shashidhara <rajaths@cs.utexas.edu> writes:
> On 07-11-2019 07:33, Aleksandar Markovic wrote:
>> I did a quick Google search on datasheets of existing RTC
>> implemtations, and the result is:
>> DS1338:
>> https://datasheets.maximintegrated.com/en/ds/DS1338-DS1338Z.pdf
>> M41T80: https://www.st.com/resource/en/datasheet/m41t80.pdf
>> M48T59: http://www.elektronikjk.pl/elementy_czynne/IC/M48T59V.pdf
>> MC146818: https://www.nxp.com/docs/en/data-sheet/MC146818.pdf
>> PL031: http://infocenter.arm.com/help/topic/com.arm.doc.ddi0224c/real_time_clock_pl031_r1p3_technical_reference_manual_DDI0224C.pdf
>> TWL92230: https://datasheet.octopart.com/TWL92230C-Texas-Instruments-datasheet-150321.pdf
>> Zynq RTC: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
>> (chapter 7)
>
> I have a few questions about this:
> [a] Is there any particular reason that you picked DS3231 ? Linux
> kernel has drivers for DS3232/34 only [1]. I did read the datasheets
> of both 3232 & 3231 and found that they are quite similar except for
> the 236 bytes of SRAM support found only in 3232.
>
> [b] As per the datasheet, DS3231 has a built-in temperature sensor.
> Temperature can be read from a dedicated register. There can be two
> approaches to emulating this: (1) Return a constant temperature value
> on every read (2) Throw a not-supported exception/warning. What is the
> qemu convention for handling such features ?
Don't throw an exception. You can at the minimum do a
qemu_log_mask(LOG_UNIMP) to indicate the system is using currently
unimplemented functionality. Alternatively wire-up a device property via
QOM so the user can vary the reported temperature.
QEMU currently doesn't have a decent API for exposing values for dynamic
emulated sensors to the outside world aside from QMP for chaning device
values. It's something we have discussed in the past but the trick is
coming up with something that can cover the wide range of device types.
Maybe QMP is good enough?
>
> [c] DS3231 also has programmable square-wave output + 32 KHz output
> pin. M41T80 chip also supports this feature. However, qemu does not
> support emulation of these features [2]. Do I take the same approach ?
>
> Thanks!
> Rajath Shashidhara
>
> References:
> [1]
> https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc/rtc-ds3232.c
> [2]
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/rtc/m41t80.c;h=914ecac8f4db418633d6daf92608cb50f6b89052;hb=HEAD
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Looking for issues/features for my first contribution
2019-11-08 9:08 ` Alex Bennée
@ 2019-11-08 13:05 ` Richard Henderson
0 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2019-11-08 13:05 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Aleksandar Markovic
On 11/8/19 10:08 AM, Alex Bennée wrote:
> Rajath Shashidhara <rajaths@cs.utexas.edu> writes:
>> [b] As per the datasheet, DS3231 has a built-in temperature sensor.
>> Temperature can be read from a dedicated register. There can be two
>> approaches to emulating this: (1) Return a constant temperature value
>> on every read (2) Throw a not-supported exception/warning. What is the
>> qemu convention for handling such features ?
>
> Don't throw an exception. You can at the minimum do a
> qemu_log_mask(LOG_UNIMP) to indicate the system is using currently
> unimplemented functionality. Alternatively wire-up a device property via
> QOM so the user can vary the reported temperature.
FWIW, Phillipe recently added a dummy temp sensor for raspi4:
git show 99c641370b52348b2893b2cd19624bf9a416ccfa
wherein he hard-codes 25C. Unless you're wanting to test the
guest kernel handling of overheating, this seems sufficient.
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-08 2:39 ` Rajath Shashidhara
2019-11-08 9:08 ` Alex Bennée
@ 2019-11-08 19:31 ` Aleksandar Markovic
2019-11-09 16:01 ` Peter Maydell
2019-11-08 19:36 ` Aleksandar Markovic
2019-11-09 19:46 ` Aleksandar Markovic
3 siblings, 1 reply; 19+ messages in thread
From: Aleksandar Markovic @ 2019-11-08 19:31 UTC (permalink / raw)
To: Rajath Shashidhara; +Cc: qemu-devel
> [a] Is there any particular reason that you picked DS3231 ? Linux kernel
> has drivers for DS3232/34 only [1]. I did read the datasheets of both
> 3232 & 3231 and found that they are quite similar except for the 236
> bytes of SRAM support found only in 3232.
>
Yes, DS3231 is a part of a board we want to support in future.
I think you could make your QEMU emulation cover both DS3231 and
DS3232 (and call your file ds323x.c, or similar). If you do it right, this may
be the model for handling similar cases, and I am sure there are plenty of
them in the whole QEMU tree. (for example, DS1307 and DS1306)
But, if this sound too complicated for you, please just cover DS3231.
Also, you can contact kernel DSxxxx drivers authors for more info.
I suspect their DS3232 driver works with DS3231 too.
Yours,
Aleksandar
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-08 19:31 ` Aleksandar Markovic
@ 2019-11-09 16:01 ` Peter Maydell
2019-11-09 21:08 ` Aleksandar Markovic
0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2019-11-09 16:01 UTC (permalink / raw)
To: Aleksandar Markovic; +Cc: Rajath Shashidhara, qemu-devel
On Fri, 8 Nov 2019 at 19:32, Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
>
> > [a] Is there any particular reason that you picked DS3231 ? Linux kernel
> > has drivers for DS3232/34 only [1]. I did read the datasheets of both
> > 3232 & 3231 and found that they are quite similar except for the 236
> > bytes of SRAM support found only in 3232.
> >
>
> Yes, DS3231 is a part of a board we want to support in future.
We should probably prefer to go with a device that's a
missing part of a board we already support -- generally
the project prefers not to take device models that don't
have a use, ie they go in with the board model rather
than before. This also means we have some way of testing
the code :-)
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-09 16:01 ` Peter Maydell
@ 2019-11-09 21:08 ` Aleksandar Markovic
2019-11-10 14:33 ` Peter Maydell
0 siblings, 1 reply; 19+ messages in thread
From: Aleksandar Markovic @ 2019-11-09 21:08 UTC (permalink / raw)
To: Peter Maydell; +Cc: Rajath Shashidhara, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1738 bytes --]
On Saturday, November 9, 2019, Peter Maydell <peter.maydell@linaro.org>
wrote:
> On Fri, 8 Nov 2019 at 19:32, Aleksandar Markovic
> <aleksandar.m.mail@gmail.com> wrote:
> >
> > > [a] Is there any particular reason that you picked DS3231 ? Linux
> kernel
> > > has drivers for DS3232/34 only [1]. I did read the datasheets of both
> > > 3232 & 3231 and found that they are quite similar except for the 236
> > > bytes of SRAM support found only in 3232.
> > >
> >
> > Yes, DS3231 is a part of a board we want to support in future.
>
> We should probably prefer to go with a device that's a
> missing part of a board we already support -- generally
> the project prefers not to take device models that don't
> have a use, ie they go in with the board model rather
> than before. This also means we have some way of testing
> the code :-)
>
>
All agreed, Peter.
However, there is an additional interesting aspect here:
DS3231 is a fairly common module for RasPi. (and, is common for some other
systems with non-arm cpus too)
I hoped that setting up RasPi emulation with newly created RTC device would
be a peace of cake, but it might be that I was wrong. I see certain
inconsistency between our command line switches "-rtc" and "-watchdog".
Using "-watchdog", a watchdog timer model can be specified, but for "-rtc",
it looks, there is no such posibility.
Given modularity of RasPi, wouldn't it be nice for end users to be able to
specify an RTC via command line?
If usage of command line is ruled out, what would be an alternative way to
integrate particular RTC support in RasPi (for any QEMU-supported RTC, not
only (for now, just envisioned) DS3231)?
Sory if I missed something big here.
Yours,
Aleksandar
> thanks
> -- PMM
>
[-- Attachment #2: Type: text/html, Size: 2466 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-09 21:08 ` Aleksandar Markovic
@ 2019-11-10 14:33 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2019-11-10 14:33 UTC (permalink / raw)
To: Aleksandar Markovic; +Cc: Rajath Shashidhara, qemu-devel
On Sat, 9 Nov 2019 at 21:08, Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
> Given modularity of RasPi, wouldn't it be nice for end users to be able to specify an RTC via command line?
The rtc option specifies properties of the backend of
an rtc device. It doesn't specify what the RTC device
exposed to the guest is. For doing that we use '-device',
but that only works for pluggable devices. In general,
we don't try to model any of that complexity for our
boards, except for the special case where there's a
nice clean interface for what the pluggable device is
(ISA, PCI, USB, etc). You could in theory come up with
a bus class which modelled RasPI pluggable modules and
then implement an RTC that way, but that's a huge amount
of work for a board model which currently is struggling
to get enough developer attention to be usable at all.
> If usage of command line is ruled out, what would be an alternative
> way to integrate particular RTC support in RasPi (for
> any QEMU-supported RTC, not only (for now, just envisioned) DS3231)?
We should just model the devices that the hardware has, so
that when you create a raspi QEMU model you get the same
things that you get if you take a stock raspi board and
power it up. The raspi board model is currently missing
lots and lots of devices, some of which are now
necessary to just be able to boot Linux. So we should
start by getting and keeping it working in a basic form,
before we even think about more obscure optional stuff
like pluggable modules.
One thing we should definitely not do is provide
a model of "some random RTC we happen to have a device
model of", just because we have a model handy. My
experience is that it's really important to stick to
a line of "just model what the real hardware does",
and not put in "QEMU does this other thing" behaviour:
it may seem convenient in the short term but it turns
into a real pain to maintain in the long term. The
actual hardware specs provide a nice clean "this is
what correct behaviour of a model should be" definition.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-08 2:39 ` Rajath Shashidhara
2019-11-08 9:08 ` Alex Bennée
2019-11-08 19:31 ` Aleksandar Markovic
@ 2019-11-08 19:36 ` Aleksandar Markovic
2019-11-08 21:54 ` BALATON Zoltan
2019-11-09 19:46 ` Aleksandar Markovic
3 siblings, 1 reply; 19+ messages in thread
From: Aleksandar Markovic @ 2019-11-08 19:36 UTC (permalink / raw)
To: Rajath Shashidhara; +Cc: qemu-devel
> [c] DS3231 also has programmable square-wave output + 32 KHz output pin.
> M41T80 chip also supports this feature. However, qemu does not support
> emulation of these features [2]. Do I take the same approach ?
Hi, Rajath.
I would rather have you amend M41T80, if there is a missing functionality.
cc-in Zoltan the creator of M41T80 emulation, for his opinion.
Yours,
Aleksandar
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-08 19:36 ` Aleksandar Markovic
@ 2019-11-08 21:54 ` BALATON Zoltan
0 siblings, 0 replies; 19+ messages in thread
From: BALATON Zoltan @ 2019-11-08 21:54 UTC (permalink / raw)
To: Aleksandar Markovic; +Cc: Rajath Shashidhara, qemu-devel
Hello,
On Fri, 8 Nov 2019, Aleksandar Markovic wrote:
>> [c] DS3231 also has programmable square-wave output + 32 KHz output pin.
>> M41T80 chip also supports this feature. However, qemu does not support
>> emulation of these features [2]. Do I take the same approach ?
>
> Hi, Rajath.
>
> I would rather have you amend M41T80, if there is a missing functionality.
>
> cc-in Zoltan the creator of M41T80 emulation, for his opinion.
The missing functionality (such as alarm timer, if I remember correctly)
could be added, but I did not bother to fully implement it because the
clients I was interested in only used the RTC part, so current
implementation is sufficient for that. Not sure if anything would use the
missing features but it would be nice to fully implement the chip.
I'm not sure what you plan to use DS3231 for but if clients only use part
of the functions then maybe implementing those first might be enough but
if you have time to correctly implement everything then that's probably
even better.
If you or someone else is still looking for a relatively easy device to
implement I have this:
https://osdn.net/projects/qmiga/wiki/SubprojectMac99I2C
The difficulty in that is that it has no docs but only open source drivers
to figure out how it works but if someone likes a challenge it may improve
mac99 emulation.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Looking for issues/features for my first contribution
2019-11-08 2:39 ` Rajath Shashidhara
` (2 preceding siblings ...)
2019-11-08 19:36 ` Aleksandar Markovic
@ 2019-11-09 19:46 ` Aleksandar Markovic
2019-11-09 21:33 ` Rajath Shashidhara
3 siblings, 1 reply; 19+ messages in thread
From: Aleksandar Markovic @ 2019-11-09 19:46 UTC (permalink / raw)
To: Rajath Shashidhara; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]
On Friday, November 8, 2019, Rajath Shashidhara <rajaths@cs.utexas.edu>
wrote:
>
> On 07-11-2019 07:33, Aleksandar Markovic wrote:
>
>>
>> I did a quick Google search on datasheets of existing RTC
>> implemtations, and the result is:
>>
>> DS1338: https://datasheets.maximintegrated.com/en/ds/DS1338-DS1338Z.pdf
>> M41T80: https://www.st.com/resource/en/datasheet/m41t80.pdf
>> M48T59: http://www.elektronikjk.pl/elementy_czynne/IC/M48T59V.pdf
>> MC146818: https://www.nxp.com/docs/en/data-sheet/MC146818.pdf
>> PL031: http://infocenter.arm.com/help/topic/com.arm.doc.ddi0224c/
>> real_time_clock_pl031_r1p3_technical_reference_manual_DDI0224C.pdf
>> TWL92230: https://datasheet.octopart.com/TWL92230C-Texas-Instruments-
>> datasheet-150321.pdf
>> Zynq RTC: https://www.xilinx.com/support/documentation/user_guides/
>> ug1085-zynq-ultrascale-trm.pdf
>> (chapter 7)
>>
>
> I have a few questions about this:
> [a] Is there any particular reason that you picked DS3231 ? Linux kernel
> has drivers for DS3232/34 only [1].
>
Hi, Rajath.
No, it doesn't. Linux kernel has a driver for DS3231. Take a closer look.
But, in any case, you base your QEMU emulation on the *datasheet*.
The OS drivers may be helpful, but they are not a reference you base your
solution on. The drivers may be obsolete, incorrect, incomplete, or just
plain wrong. Additionally, as QEMU, of course, supports emulation of
systems running variety of OSs, the existence of the Linux kernel driver is
not a necessary condition for QEMU implementation. QEMU emulates many
systems that Linux never ran on, and could not be run at all.
Sincerely,
Aleksandar
>
> I did read the datasheets of both 3232 & 3231 and found that they are
> quite similar except for the 236 bytes of SRAM support found only in 3232.
>
> [b] As per the datasheet, DS3231 has a built-in temperature sensor.
> Temperature can be read from a dedicated register. There can be two
> approaches to emulating this: (1) Return a constant temperature value on
> every read (2) Throw a not-supported exception/warning. What is the qemu
> convention for handling such features ?
>
> [c] DS3231 also has programmable square-wave output + 32 KHz output pin.
> M41T80 chip also supports this feature. However, qemu does not support
> emulation of these features [2]. Do I take the same approach ?
>
> Thanks!
> Rajath Shashidhara
>
> References:
> [1] https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc
> /rtc-ds3232.c
> [2] https://git.qemu.org/?p=qemu.git;a=blob;f=hw/rtc/m41t80.c;h=
> 914ecac8f4db418633d6daf92608cb50f6b89052;hb=HEAD
>
[-- Attachment #2: Type: text/html, Size: 4595 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Looking for issues/features for my first contribution
2019-11-09 19:46 ` Aleksandar Markovic
@ 2019-11-09 21:33 ` Rajath Shashidhara
2019-11-09 23:55 ` Aleksandar Markovic
0 siblings, 1 reply; 19+ messages in thread
From: Rajath Shashidhara @ 2019-11-09 21:33 UTC (permalink / raw)
To: Aleksandar Markovic; +Cc: qemu-devel
On 11/9/19 1:46 PM, Aleksandar Markovic wrote:
>
>
> Hi, Rajath.
>
> No, it doesn't. Linux kernel has a driver for DS3231. Take a closer look.
Kernel driver found here:
https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc/rtc-ds3232.c
did register NVMEM of 236 bytes with the kernel. As long as this NVMEM
is never accessed, the same driver should work for both DS3231 and DS3232.
Is there any other driver you are referring to ? Please let me know if I
missed something here.
>
> But, in any case, you base your QEMU emulation on the *datasheet*.
>
> The OS drivers may be helpful, but they are not a reference you base
> your solution on. The drivers may be obsolete, incorrect, incomplete, or
> just plain wrong. Additionally, as QEMU, of course, supports emulation
> of systems running variety of OSs, the existence of the Linux kernel
> driver is not a necessary condition for QEMU implementation. QEMU
> emulates many systems that Linux never ran on, and could not be run at all.
>
I was only looking at the Kernel drivers to setup a test framework. I
plan to test my implementation using a Raspi emulation with qemu,
configure it with a DS3231 device. After this, I should be able to use
ioctl() on /dev/rtc to test the functionality:
https://linux.die.net/man/4/rtc
If you have a better approach to testing, do let me know.
Thanks,
Rajath Shashidhara
^ permalink raw reply [flat|nested] 19+ messages in thread
* Looking for issues/features for my first contribution
2019-11-09 21:33 ` Rajath Shashidhara
@ 2019-11-09 23:55 ` Aleksandar Markovic
0 siblings, 0 replies; 19+ messages in thread
From: Aleksandar Markovic @ 2019-11-09 23:55 UTC (permalink / raw)
To: Rajath Shashidhara; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
On Saturday, November 9, 2019, Rajath Shashidhara <rajaths@cs.utexas.edu>
wrote:
>
>
> On 11/9/19 1:46 PM, Aleksandar Markovic wrote:
>
> >
> >
> > Hi, Rajath.
> >
> > No, it doesn't. Linux kernel has a driver for DS3231. Take a closer look.
>
> Kernel driver found here:
> https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc/rtc-ds3232.c
> did register NVMEM of 236 bytes with the kernel. As long as this NVMEM
> is never accessed, the same driver should work for both DS3231 and DS3232.
>
> Is there any other driver you are referring to ? Please let me know if I
> missed something here.
>
>
The official DS3231 driver is (hidden) in:
https://elixir.bootlin.com/linux/v5.4-rc6/source/drivers/rtc/rtc-ds1307.c
This driver actually supports around 15 RTC chips, and DS3231 among them.
It contains fairly sophisticated "feature control" that enables it to
support multiple RTC chips.
> >
> > But, in any case, you base your QEMU emulation on the *datasheet*.
> >
> > The OS drivers may be helpful, but they are not a reference you base
> > your solution on. The drivers may be obsolete, incorrect, incomplete, or
> > just plain wrong. Additionally, as QEMU, of course, supports emulation
> > of systems running variety of OSs, the existence of the Linux kernel
> > driver is not a necessary condition for QEMU implementation. QEMU
> > emulates many systems that Linux never ran on, and could not be run at
> all.
> >
>
> I was only looking at the Kernel drivers to setup a test framework. I
> plan to test my implementation using a Raspi emulation with qemu,
> configure it with a DS3231 device. After this, I should be able to use
> ioctl() on /dev/rtc to test the functionality:
> https://linux.die.net/man/4/rtc
>
> If you have a better approach to testing, do let me know.
>
>
Your method sounds good to me. Hopefully it won't be too difficult to you
to execute it.
Best wishes,
Aleksandar
> Thanks,
> Rajath Shashidhara
>
[-- Attachment #2: Type: text/html, Size: 3021 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread