ATmega64 Watchdog Timer

Go To Last Post
31 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quick easy question: I want to enable the watchdog timer on an Atmega64 in System Reset Mode, with a timeout period of 4 seconds. Can someone confirm that all I need to do for that to work is to set the WDTON fuse and define the timout period using the code below?:

 

// Watchdog Timer initialization
// Watchdog Timer Prescaler: OSC/512k (4s timeout = longer than program execution time)
#pragma optsize-
WDTCR=(1<<WDCE) | (1<<WDE) | (1<<WDP2) | (0<<WDP1) | (1<<WDP0);
WDTCR=(0<<WDCE) | (1<<WDE) | (1<<WDP2) | (0<<WDP1) | (1<<WDP0);
#ifdef _OPTIMIZE_SIZE_
#pragma optsize+
#endif

 

I've read the datasheet as well as this Atmel app note:
http://www.atmel.com/Images/doc2...

But I still don't feel entirely clear on it, and would like to get further confirmation.

 

Last Edited: Thu. Feb 2, 2017 - 10:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
all I need to do for that to work is to set the WDTON fuse and define the timout period using the code below?:

That might be a tricky question, at least for me.  Yes, I use watchdog in all my AVR apps -- but have never used the WDTON fuse.  Depending on the model, one needs to take care with sequences to set the timeout value, as well as protecting against cascading resets as after a WD reset it goes again with the lowest timeout value.

 

It looks like CodeVision code?  What version? 

 

Without bit names ;) and without WDTON, this is a working fragment from one of my production apps:

// Watchdog Timer initialization
// Watchdog Timer Prescaler: OSC/1024k
#pragma optsize-
WDTCR=0x1E;
WDTCR=0x0E;
#ifdef _OPTIMIZE_SIZE_
#pragma optsize+
#endif

 

 

 

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Thu. Feb 2, 2017 - 10:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks! Looks like your code is basically the same. The reason I thought to enable the WDTON fuse is due to the following text that's in the Atmel app note I referenced in my first post:

 

When using the Enhanced Watchdog Timer it is important to know that if the Watchdog Always On (WDTON) fuse is programmed, the only possible operation mode is WDT System Reset Mode. This security feature prevents software from enabling the WDT Interrupt Mode unintentionally, which could disable the WDT System Reset functionality.

and

Configuring the WDT to work as a system reset source only, is straightforward. Enable the WDT System Reset Mode, set a reasonable timeout delay and off you go.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Incidentally, after I put this stuff into my code how do I go about testing it? The application I'm running is quite simple, and has so far run without a glitch for over a year now without a watchdog in place. I don't really know how I can simulate a hung processor so that the watchdog kicks in. Or am I supposed to just believe that it's going to work? Maybe there's some code I can put in to intentionally hang the processor and see if the watchdog resets the chip?

Last Edited: Fri. Feb 3, 2017 - 05:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well:

for(;;);

or

while(1);

scattered around your code at places where it "might" get stuck would be a test. When it gets "stuck" in such a loop, after the WDT period it should trigger.

 

Of course this is just "faking it". What you are really looking for is things like:

while (!(UCSRA & (1 << UDRE)));

where you do something like waiting for a UART character to arrive - usually it does and the code moves on but on the occasion that someone cuts through the cable feeding the UART the delivery of characters dries up and suddenly this wait becomes infinite. Well designed code might have some kind of timeout already wrapped around this in case such an unlikely event occurs but you may not have planned for every eventuality - in which case the watchdog should dig you out of the hole.

 

(of course the code may restart/recover only to re-encounter the same problem because that cable is still cut!).

 

So what you are looking for are generally circumstances where a periodic wait might become an infinite wait in unusual circumstances. Or perhaps some combination of inputs that drags you off into that switch/case condition you never really expected to happen or whatever. You can just put in a real "infinite loop" to trigger what might occur if such "stuck" conditions ever actually occurred.
 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
I can put in to intentionally hang the processor and see if the watchdog resets the chip?

 

That's how it is done!  Note: when the WDT reset occurs, it will reset the timeout to the shortest time, so one of the first things you need to do in your program is set the correct WDT timeout time or disable the WDT until you do. 

 

Jim

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ki0bk wrote:

That's how it is done!  Note: when the WDT reset occurs, it will reset the timeout to the shortest time, so one of the first things you need to do in your program is set the correct WDT timeout time or disable the WDT until you do. 

 

And the code that's in my initial post will do that, right?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

Well:

for(;;);

or

while(1);

scattered around your code at places where it "might" get stuck would be a test. When it gets "stuck" in such a loop, after the WDT period it should trigger.

 

Of course this is just "faking it". What you are really looking for is things like:

while (!(UCSRA & (1 << UDRE)));

where you do something like waiting for a UART character to arrive - usually it does and the code moves on but on the occasion that someone cuts through the cable feeding the UART the delivery of characters dries up and suddenly this wait becomes infinite. Well designed code might have some kind of timeout already wrapped around this in case such an unlikely event occurs but you may not have planned for every eventuality - in which case the watchdog should dig you out of the hole.

 

(of course the code may restart/recover only to re-encounter the same problem because that cable is still cut!).

 

So what you are looking for are generally circumstances where a periodic wait might become an infinite wait in unusual circumstances. Or perhaps some combination of inputs that drags you off into that switch/case condition you never really expected to happen or whatever. You can just put in a real "infinite loop" to trigger what might occur if such "stuck" conditions ever actually occurred.
 

 

This is confusing to me, so let me ask a couple of questions:

  • The code I posted in my initial post; that's a watchdog initialization. It only gets read once, when the processor powers up, right?
  • If that's true, why would a watchdog ever trigger on an infinite while loop or while sitting waiting for a UART character? The processor isn't actually hung up, it's still in its main loop. I don't understand why that would trigger the watchdog. Even though the processor is waiting, it's still doing its job.
  • If there's no code in the main program loop to ping the internal watchdog and reset the counter (which, judging from what I've read and from the responses in this thread, I apparently don't need), then where is that ping coming from?
Last Edited: Fri. Feb 3, 2017 - 05:48 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

From #1 it's "either/or". Either you set WDTON fuse in which case (apart from changing the period) you don't need to do anything else - the watchdog now starts ticking at power on. Or you do the writes to WDCR to enable it in code where it's not on already.

 

Of course this is only part of the job. What happens when it is enabled is that the WDT timer counts down. When it reaches 0 it resets the AVR (on later models you have the option of an interrupt first). So if you only set WDTON or add some lines to enable it to your code it will keep resetting the AVR.

 

The way you use a watchdog is you then look through your code and identify where the "long loops" might be. Perhaps that while loop waiting for a UARt character (for example) could stick there for a while? So instead of just:

while (!(UCSRA & (1 << UDRE)));

what you actually now need is:

while (!(UCSRA & (1 << UDRE))) {
    asm("WDR");
}

(your compiler may have a "cleaner" way to do that) but the point is that if you expect a reasonably long wait, during which the WDT might actually count down as far as 0 you need to execute the AVR's WDR opcode from time to time. Each time it executes the WDT timer is reset back to the start of the count-down. So it never reaches 0. It's then in code such as:

while(1) {
    // nothing to see here
}

that the WDT will come into action., As this loop has nothing executing WDRs the WDT will just carry on counting down and when it reaches 0 it will pull the AVR into reset.

 

To be honest, WDT is not something you really add at the end as an after thought. If you want a product with watchdog protection you really need to design it in from the start. As you go you need to indentify where all the "long loops" are and pepper them with WDRs. If, as you appear to be doing here, you just have a block of existing code, all you do is "turn it on" and do nothing about considering where the WDRs need to be for the "expected path" you are going to find your AVR resetting "unexpectedly" as it goes into that one "long condition" you hadn't thought of and had not protected with WDRs.

 

So look at using this tool in your next design - not trying to retro-fit it it to an existing one.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Some where in the main loop you will need a WDR();  call to feed the dog.

When the dog has not been feed for the time out period, he barks and resets the mcu!

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Our posts just crossed but I think I just answered your #8 in my #9 anyway.

 

You clearly don't know what the watchdog is or is used for if you don't know about WDRs and why you need them all over your code.

 

Of course there are some "clean" designs that have exactly one "WDR" in them - the code is designed so that no path should ever take anywhere near the WDT timeout time - it always gets back to a central loop/scheduler "in time" and then that scheduler execuctes the one WDR before it heads off into the next branch of the code that should also get back in time. On the unexpected occasion it calls off to some operation and that does actually take longer than the WDT period and no WDR is executed in the meantime then the AVR resets - which was the plan.

 

So it may do something like:

schedler: WDR - WDT starts to count down from 100 (say)
scheduler: call taskA
taskA: do something - WDT counts down 99, 98, 97..
taskA: ends
schduler: WDR - WDT back to 100
scheduler: call taskB
taskB: do someting - WDT counts down 99, 98 ... 65, 64, 63..
taskB: ends
scheduler: WDR - WDT to 100
scheduler: call taskC
taskC: start job 99, 98, 97 .. 333, 32, 31...
taskC: something terrible happens - not getting results expected
taskC: 30, 29 .... 7, 6, 5, 4, 3, 2 , 1
WDT: Shite - I just reached 0 - RESET AVR

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ki0bk wrote:
call to feed the dog.
That's the nice term - a lot of people call the operation of WDR (or equivalent) "kicking the dog". If you don't kick him often enough he gets real nasty and resets your CPU!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks everyone, that's the piece I was missing - the WDR call in the main loop. Much appreciated!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
the WDR call in the main loop.
In the main loop or any other code path where you could be delayed for a while. That's the thing - if you haven't designed this in from the start there may be some path that in some circumstance takes longer than you expected and in that time WDT counts to 0. Then BOOM!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

You clearly don't know what the watchdog is or is used for if you don't know about WDRs and why you need them all over your code.

 

I originally assumed there was separate watchdog hardware within the chip and that some hidden subroutine tied to the main CPU but independent of my code pinged the watchdog periodically. In such a configuration, the watchdog would only timeout if the CPU fully hung, and would never activate if the code was stuck somewhere in the main loop. This would have been just fine with me, provided that the timeout period was long enough. The first response to my question seemed to confirm my suspicions - just set the fuse bit and set the time you need once; if the CPU hangs, the watchdog will reset the chip - not dependent on the main loop at all.

 

But then you posted code that tested the watchdog by hanging the main loop, at which point I posed the question:

If there's no code in the main program loop to ping the internal watchdog and reset the counter (which, judging from what I've read and from the responses in this thread, I apparently don't need), then where is that ping coming from?

So in fact it did seem highly strange to me that, if the watchdog relied on the main loop for its ping, that nobody had mentioned I needed to kick the dog from within it. Until post #10.

Last Edited: Fri. Feb 3, 2017 - 06:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
I don't really know how I can simulate a hung processor so that the watchdog kicks in.

 

 

lavadisco wrote:
If there's no code in the main program loop to ping the internal watchdog and reset the counter (which, judging from what I've read and from the responses in this thread, I apparently don't need), then where is that ping coming from?

I don't know.  Perhaps your app was constantly resetting?

 

A re-examination of the datasheet might help now:

Watchdog Timer The Watchdog Timer is clocked from a separate On-chip Oscillator which runs at 1 Mhz. This is
the typical value at VCC = 5V. See characterization data for typical values at other VCC levels. By
controlling the Watchdog Timer prescaler, the Watchdog Reset interval can be adjusted as
shown in Table 22 on page 58. The WDR – Watchdog Reset – instruction resets the Watchdog
Timer. The Watchdog Timer is also reset when it is disabled and when a Chip Reset occurs.
Eight different clock cycle periods can be selected to determine the reset period. If the reset
period expires without another Watchdog Reset, the ATmega64 resets and executes from the
Reset Vector. For timing details on the Watchdog Reset, refer to page 55.
To prevent unintentional disabling of the Watchdog or unintentional change of Time-out period,
three different safety levels are selected by the fuses M103C and WDTON as shown in Table
21. Safety level 0 corresponds to the setting in ATmega103. There is no restriction on enabling
the WDT in any of the safety levels. Refer to “Timed Sequences for Changing the Configuration
of the Watchdog Timer” on page 60
for details.
 

So as I mentioned earlier, there may be a different sequence needed if indeed you have WDTON fuse programmed. Note the the CV prologue does a WDR and flag clear immediately at reset, avoiding the race that might occur with a long prologue.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Fri. Feb 3, 2017 - 07:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
n the main loop or any other code path where you could be delayed for a while. That's the thing - if you haven't designed this in from the start there may be some path that in some circumstance takes longer than you expected and in that time WDT counts to 0. Then BOOM!

 

Generally, I'll agree.

 

There are those that will state: "App should have only one WDR."  Not a bad guideline, but in practice can create some contortions.  Suppose for example that the app has 128ms WD interval set.  And it has "restore factory defaults" feature that copies 200 bytes of EEPROM at a couple milliseconds each.  Bark goes the dog.  So if the dogma/guideline is followed, the code must implement a series of tasks or states to do a chunk at a time.  IMO more error prone and larger and in the end slower.  So toss in a WDR, do an appropriate chunk, another WDR, another chunk, with straightforward code in one place.

 

IMO/IME the [hopefully] single WDR in the main loop should be further qualified such that all important periodic activities are still active.  The main loop may appear very happy even if interrupt aren't firing.  So the WDR should be done after qualifying that all important periodic events are still firing.

 

[edit] could have saved some typing...

https://www.avrfreaks.net/comment...

theusch wrote:

There are prior extensive threads on this.  Example: https://www.avrfreaks.net/forum/w...

where I said:

My general approach is to set a flag when regularly-occurring "important" events happen. Examples include timer and ADC ISRs. Some systems expect a poll from the master within a short time; another "important" event. Then, I check the "AND" of all the important flags in the main loop and if true, clear them all.

... and other pertinent comments from me and others.

 

So timer tick sets a flag, and the main loop acts on that flag.  There is probably the most common place for me to do my WDR -- the main loop is still cycling, and interrupts are still running.

 

Note the AND in that.  One might add to that, as per examples in the linked thread.  If I expect a poll from a master, then that is another flag to AND.  One might also AND with the flag from ADC interrupts. 

 

In your case it might appear that somewhere in non-interrupt code, an infinite loop occurred.
 

 

That whole thread is pertinent, along with the linked thread [where the single WDR dogma is explored as well].

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Fri. Feb 3, 2017 - 07:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I set up my watchdog timer the way I want it, but now I'm trying to figure out what the reset command is (in CodeVisionAVR) that goes into my application to reset the watchdog. I tried all of these in my main loop:

 

wdt_reset();
_WDR();
asm("WDR");

 

But none compile in CodeVision. Anybody know what the correct CAVR syntax for doing this? I've asked in the CVAVR forums, but no reponse yet.

Last Edited: Mon. Mar 27, 2017 - 08:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
asm("WDR");

Perhaps #asm ("wdr")

?

 

In the CV manual/Help:

 

In case the watchdog timer is enabled, you must include yourself the appropriate code sequences to reset it periodically. Example:

 

#asm("wdr")

 

For more information about the watchdog timer you must consult the Atmel data sheet for the chip that you use.

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Mon. Mar 27, 2017 - 08:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

 

Perhaps #asm ("wdr")

?

 

Yeah, that compiled. Thanks! Will test it...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Resurrecting an old thread of mine. So I've got some code running on an Atmega128, with the following:

 

- WDTON is programmed (Watchdog timer enabled)

- I have this code up at the top in the initializations:

// Watchdog Timer initialization
// Watchdog Timer Prescaler: OSC/512k = 3.9s timeout period @ 2MHz
#pragma optsize-
#asm("wdr")
WDTCR|=(1<<WDCE) | (1<<WDE);
WDTCR=(0<<WDCE) | (1<<WDE) | (1<<WDP2) | (0<<WDP1) | (1<<WDP0);
#ifdef _OPTIMIZE_SIZE_
#pragma optsize+
#endif

 

- And this code at the bottom of my deterministically-timed one-second long loop:

#asm("wdr");

 

I know the code is 1s long because I have a timer counting to 1s that spits out a telemetry stream every time through, and I've timed the frequency of the output. It's highly reliable.

 

Yet when I enable the watchdog timer, the telemetry stops coming out and the chip appears to be in continuout reset. Why would a watchdog programmed to go off after four seconds reset in a 1s long while loop?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The Watchdog clock runs at 1MHz on the ATmega128, so it only takes 1/2 second

for it to count up to 512k.  The maximum delay you can get by setting the prescaler

to 2048k is about 2 seconds.

 

--Mike

 

ATmega128 Watchdog Prescale Settings

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks! Sounds like I should set it to 2048k and put an extra dog kick halfway through just in case.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Should we add "kick the dog" to the list of banned phrases along with master/slave?  :-)

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, just tried it with and without WDTON programmed. Works fine with WDTON unprogrammed, but with it programmed the chip goes into reset. It seems like if WDTON is programmed, that my timeout setting is ignored. For now I'll leave it in with WDTON unprogrammed, but I would like to better understand what the advantages/disadvantages and differences are of programming WDTON vs not. The section in the datasheet on this topic is confusing to me.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
It seems like if WDTON is programmed, that my timeout setting is ignored.

Have you really studied the datasheet information on watchdog?

 

Where did you come up with

lavadisco wrote:
OSC/512k = 3.9s timeout period @ 2MHz

...in the first place?

 

Does your code follow:

In safety level 2, it is not possible to disable the Watchdog Timer, even with the algorithm
described above. See “Timed Sequences for Changing the Configuration of the Watchdog
Timer” on page 60.

 

 

I'm not a WDTON persom.  I've lost track -- CV, right?  The prologue has a "disable" sequence that won't work with WDTON per the above.  Does the CV WDT setup sequence follow the page 60 rules?  Dunno; I'd think so.  The Wizard's |= is a bit disconcerting.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Thu. Sep 13, 2018 - 08:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

Have you really studied the datasheet information on watchdog?

 

Depends - does reading it and poorly comprehending it count? Because I did that.

 

Quote:
Where did you come up with

lavadisco wrote:
OSC/512k = 3.9s timeout period @ 2MHz

...in the first place?

 

I thought OSC was referring to the system OSC, not the watchdog clock. I am running at 2MHz, so that's what I used. I've since been made aware that the watchdog clock runs at 1MHz.

 

Quote:
Does your code follow:

In safety level 2, it is not possible to disable the Watchdog Timer, even with the algorithm
described above. See “Timed Sequences for Changing the Configuration of the Watchdog
Timer” on page 60.

 

 

Yes, I read that, but in that section they don't address the role of WDTON.

 

Quote:

I'm not a WDTON persom.  I've lost track -- CV, right?  The prologue has a "disable" sequence that won't work with WDTON per the above.  Does the CV WDT setup sequence follow the page 60 rules?  Dunno; I'd think so.  The Wizard's |= is a bit disconcerting.

 

Just to reiterate, the watchdog is working the way I want it to now, just had to change the timout a bit to account for the 1MHz watchdog clock. Now I'm just trying to figure out when WDTON is necessary/useful. I'm asking because it seems like the way I have it now, I'm relying on my own software to enable the watchdog, wheras enabling it using WTDON at least on the surface would seem to be a more independent method of enabling that might not be subject to my software?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
Yes, I read that, but in that section they don't address the role of WDTON.
???

theusch wrote:
Have you really studied the datasheet information on watchdog?
lavadisco wrote:
I thought OSC was referring to the system OSC, not the watchdog clock.

lavadisco wrote:
Yes, I read that, but in that section they don't address the role of WDTON.

So, what is Safety Level 2?  Does the datasheet mention it?

But indeed, I am not a WDTON person.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

Yes. I missed the 1MHz oscillator thing, thanks for proving that I missed it. I did read it, must not have sunk in. Not trying to be sneaky and take your hard-earned knowledge without putting effort in, things are not as obvious to me when I read them as they are to you because I'm sure you've got a lot more experience with all of this.

 

 

I've read all that. Let me break it down the way I understand it and you can throw darts at it.

 

I have not programmed M103C or WDTON. So I must be at Safety Level 1 where the watchdog can be disabled and have its timeout changed with a "Timed Sequence" (no clue what that means - is the code I put in to set the timout considered a "Timed Sequence"?). If I programmed WDTON, then the watchdog would be "Always Enabled", with timeout changed using a "Timed Sequence".

 

The only thing this tells me about why I would want to use WDTON is that using it ensures that the watchdog can't be turned off? Is that the only reason why? Or is there something else? This is the fundamental question I am asking, why would someone choose to use WDTON?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, the WDTON fuse has only one purpose, to make sure you can't accidentally turn off

the watchdog timer.  This could be important if you make devices that could cause injury

or damage if something goes wrong.  Or your spacecraft might burn all it's fuel because

some valve got stuck open while the ground crew was trying to send a software reset

signal from millions of miles away....

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

lavadisco wrote:
I have not programmed M103C or WDTON.

I thought you said earlier that it was working without WDTON, but was not working >>with<< WDTON.  I must have misread.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.