Skip to content

Commit

Permalink
Fix convertReset polarity change (#2548)
Browse files Browse the repository at this point in the history
The `convertReset` function did not account for the possibility that the
two domains might not have the same reset polarity.
  • Loading branch information
DigitalBrains1 authored Jul 18, 2023
1 parent 4755ced commit fc20d10
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog/2023-07-17T18_31_48+02_00_convert_reset
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FIXED: When `convertReset` was used with two domains that had a different reset polarity, the polarity of the signal was not changed.
14 changes: 10 additions & 4 deletions clash-prelude/src/Clash/Explicit/Reset.hs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{-|
Copyright : (C) 2020-2021, QBayLogic B.V.
Copyright : (C) 2020-2023, QBayLogic B.V.,
2022 , Google LLC
License : BSD2 (see the file LICENSE)
Maintainer : QBayLogic B.V. <devops@qbaylogic.com>
Expand Down Expand Up @@ -278,10 +278,16 @@ convertReset
-> Clock domB
-> Reset domA
-> Reset domB
convertReset clkA clkB rstA0 = rstA2
convertReset clkA clkB rstA0 = rstB1
where
rstA1 = unsafeToReset (unsafeSynchronizer clkA clkB (unsafeFromReset rstA0))
rstA1 = unsafeFromReset rstA0
rstA2 =
case (resetPolarity @domA, resetPolarity @domB) of
(SActiveLow, SActiveLow) -> rstA1
(SActiveHigh, SActiveHigh) -> rstA1
_ -> not <$> rstA1
rstB0 = unsafeToReset $ unsafeSynchronizer clkA clkB rstA2
rstB1 =
case (sameDomain @domA @domB) of
Just Refl -> rstA0
Nothing -> resetSynchronizer clkB rstA1
Nothing -> resetSynchronizer clkB rstB0
14 changes: 12 additions & 2 deletions clash-prelude/tests/Clash/Tests/Reset.hs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import Test.Tasty.HUnit
import Test.Tasty.TH
import Clash.Explicit.Prelude

import qualified Prelude as P

-- Testing with explicit declaration of the Low type alias
type Low = ("Low" :: Domain)
createDomain vSystem{vName="Low", vResetPolarity=ActiveLow}

createDomain vSystem{vName="NoInit", vInitBehavior=Unknown}

sampleResetN :: KnownDomain dom => Int -> Reset dom -> [Bool]
sampleResetN n = sampleN n . unsafeToActiveHigh

Expand All @@ -41,6 +41,16 @@ case_onePeriodGlitch_LowPolarity =
[True,True,True,True,False,False,False,False,False,True,True,False]
@=? sampleResetN 12 (resetGlitchFilter d2 (clockGen @Low) onePeriodGlitchReset)

-- Check that the meaning of @Reset@ is maintained when converting from
-- active-low to active-high.
case_convertReset_polarity_change :: Assertion
case_convertReset_polarity_change =
-- In domains with synchronous resets and defined initial values,
-- @resetSynchronizer@ will start with an asserted reset.
True : True : P.replicate 8 False
@=? sampleResetN 10 (convertReset (clockGen @Low) (clockGen @System)
(resetFromList $ P.repeat False))

tests :: TestTree
tests = testGroup "Reset"
[ $(testGroupGenerator)
Expand Down

0 comments on commit fc20d10

Please sign in to comment.