Ask which chip it is, and which layout goes on it

app/controllers/cameras/socs_controller.rb

@@ -86,6 +86,7 @@ def update
       @camera.camera_mac_address = permitted_params[:camera_mac_address].to_s.downcase.gsub('-', ':')
       @camera.server_ip_address = permitted_params[:server_ip_address]
       @camera.flash_type = permitted_params[:flash_type]
+      @camera.partition_layout = permitted_params[:partition_layout]
       @camera.firmware_version = permitted_params[:firmware_version]
       @camera.network_interface = permitted_params[:network_interface]
       @camera.sd_card_slot = permitted_params[:sd_card_slot]
@@ -112,13 +113,20 @@ def update
       # query string and the commands.
       @camera.flash_type = @camera.soc.default_flash_chip unless @camera.flash_type.in?(Camera::FLASH_CHIP)
 
-      # to handle nor32m size still using nor16m command. After the default
-      # above, not before: the commands name the chip, so reading the flash type
-      # first left the page telling a 16MB camera to `run urnor16m` and then
-      # erasing from the 8MB overlay offset, 733,184 bytes into what it had just
-      # written. That is the failure #60 described, by another route.
-      @flash_type_command = @camera.flash_type
-      @flash_type_command = 'nor16m' if @camera.flash_type.eql?('nor32m')
+      # The bootloader macros are named after the layout, not the chip. This
+      # used to be the flash type with `nor32m` rewritten to `nor16m`, which is
+      # the same answer for every combination the menu could then produce --
+      # there is no mtdpartsnor32m anywhere upstream, so a 32MB part has always
+      # worn the 16MB layout. Camera#partition_layout says it directly now, and
+      # says it for the 8MB-layout-on-a-larger-chip case too.
+      #
+      # After the flash type has settled, not before: the layout defaults to
+      # the chip's own, so reading it first left the page telling a 16MB camera
+      # to `run urnor16m` and then erasing from the 8MB overlay offset, 733,184
+      # bytes into what it had just written. That is the failure #60 described,
+      # by another route.
+      warn_if_layout_changed permitted_params[:partition_layout]
+      @flash_type_command = @camera.partition_layout
 
       if @vendor.name.eql?("SigmaStar") && @camera.flash_type.eql?("nand")
         render 'cameras/socs/sigmastar_nand_is_weird'
@@ -152,12 +160,15 @@ def update
     end
 
     def download_full_image
-      permitted_params = params.permit(:id, :vendor_id, :flash_size, :fw_release, :flash_type)
+      permitted_params = params.permit(:id, :vendor_id, :flash_size, :fw_release, :flash_type, :layout)
       flash_size = permitted_params[:flash_size]
       flash_type = permitted_params[:flash_type]
       fw_release = permitted_params[:fw_release]
       @soc = Soc.find(params[:id])
-      fw = Firmware.new(size: flash_size, flash_type: flash_type, release: fw_release, soc: @soc)
+      # An absent layout means the chip's own, which is what every link written
+      # before the wizard could tell the two apart meant.
+      fw = Firmware.new(size: flash_size, flash_type: flash_type, release: fw_release, soc: @soc,
+                        layout: permitted_params[:layout])
       fw.generate
       # Recorded here rather than in Firmware, because a cached image is sent
       # without being rebuilt and it is the sending that is worth counting.
@@ -236,6 +247,7 @@ def full_list
     # between a key and a field and a table is where that is visible. Insertion
     # order is the precedence: `var` is applied first so `ver` overwrites it.
     PERMALINK_FIELDS = { cip: :camera_ip_address, sip: :server_ip_address, rom: :flash_type,
+                         part: :partition_layout,
                          var: :firmware_version, ver: :firmware_version,
                          net: :network_interface, sd: :sd_card_slot }.freeze
 
@@ -278,7 +290,7 @@ def narrow_to_what_the_menu_offers(camera)
       # published as Ultimate and nothing else, hi3516cv6xx and hi3519dv500,
       # keeps it, because naming a Lite tarball upstream never built is worse
       # than the size warning `update` will give.
-      return unless camera.flash_type.eql?('nor8m') && camera.firmware_version.eql?('ultimate')
+      return unless camera.partition_layout.eql?('nor8m') && camera.firmware_version.eql?('ultimate')
       return unless camera.soc.available_releases('nor').include?('lite')
 
       camera.firmware_version = 'lite'
@@ -318,7 +330,7 @@ def missing_asset_message(soc)
     # download link for `flash_size=8&fw_release=ultimate`, and `run uknor8m;
     # run urnor8m` -- with no indication that none of it can work.
     def enforce_eight_meg_limit
-      return unless @camera.flash_type.eql?('nor8m') && @camera.firmware_version.eql?('ultimate')
+      return unless eight_meg_rootfs_with_ultimate?
 
       published = @camera.soc.available_releases('nor')
       # Nothing on NOR at any size is a different problem, and
@@ -328,13 +340,45 @@ def enforce_eight_meg_limit
 
       if published.include?('lite')
         @camera.firmware_version = 'lite'
-        flash.now[:warning] = '8MB Flash ROM can only be flashed with Lite or FPV edition!'
+        flash.now[:warning] = eight_meg_warning
       else
-        flash.now[:alert] =
-          'The Ultimate edition does not fit an 8MB flash chip, and OpenIPC publishes no Lite build ' \
-          'for this SoC on NOR. These instructions cannot produce a working camera on 8MB flash -- ' \
-          'this SoC needs a larger chip.'
+        flash.now[:alert] = no_lite_for_eight_meg_alert
+      end
+    end
+
+    # The layout, not the chip: the 8MB one gives the rootfs 5120KB wherever it
+    # is written, and that is what Ultimate does not fit in.
+    def eight_meg_rootfs_with_ultimate?
+      @camera.partition_layout.eql?('nor8m') && @camera.firmware_version.eql?('ultimate')
+    end
+
+    # "This SoC needs a larger chip" is the right advice for an 8MB part and the
+    # wrong advice for a 16MB one wearing the 8MB layout, where the chip is
+    # already big enough and the layout is the thing to change. The guard above
+    # reaches both since it became the layout's, so this has to tell them apart
+    # too -- it is the branch for a SoC published as Ultimate and nothing else,
+    # hi3516cv6xx and hi3519dv500, where there is no Lite to fall back to.
+    def no_lite_for_eight_meg_alert
+      unless @camera.flash_type.eql?('nor8m')
+        return 'The Ultimate edition does not fit the 8MB partition layout, and OpenIPC publishes ' \
+               'no Lite build for this SoC on NOR. Choose the 16MB layout, which this chip is big ' \
+               'enough for.'
       end
+
+      'The Ultimate edition does not fit an 8MB flash chip, and OpenIPC publishes no Lite build ' \
+        'for this SoC on NOR. These instructions cannot produce a working camera on 8MB flash -- ' \
+        'this SoC needs a larger chip.'
+    end
+
+    # The chip when the chip is what limits them, and the layout when it is the
+    # layout: a 5120KB rootfs partition is a 5120KB rootfs partition whether the
+    # part around it is 8MB or 32MB, and on the larger ones there is something
+    # the reader can actually do about it.
+    def eight_meg_warning
+      return '8MB Flash ROM can only be flashed with Lite or FPV edition!' if @camera.flash_type.eql?('nor8m')
+
+      'The 8MB partition layout leaves 5MB for the rootfs, which only the Lite and FPV editions ' \
+        'fit. Choose the 16MB layout to install Ultimate on this chip.'
     end
 
     # Nothing published for the chip that was chosen, at any edition.
@@ -352,6 +396,22 @@ def enforce_eight_meg_limit
     # available_releases answers the known list rather than [] when the index
     # cannot be read, so an unreachable index does not turn into "OpenIPC
     # publishes nothing for this SoC".
+    # The layout is refused rather than clamped in silence when the chip cannot
+    # hold it -- reachable only from a hand-edited query string, since the menu
+    # does not offer the 16MB layout on an 8MB part, but it decides where the
+    # rootfs is written and a page that quietly showed the other one would be
+    # describing a different install from the one that was asked for.
+    def warn_if_layout_changed(asked)
+      # Unrecognised is the same as unset, as it is everywhere else here, and
+      # NAND has one layout and no menu to choose it from.
+      return if @camera.nand? || !asked.in?(Camera::PARTITION_LAYOUT)
+      return if asked.eql?(@camera.partition_layout)
+
+      flash.now[:warning] =
+        'The 16MB partition layout needs a 16MB chip -- its rootfs alone ends at 0xD50000. ' \
+        "Showing the #{@camera.layout_size}MB layout instead."
+    end
+
     def warn_if_nothing_published
       return if @camera.soc.available_releases(@camera.flash_type_type).any?
 
@@ -362,7 +422,7 @@ def warn_if_nothing_published
 
     def permitted_params
       params.require(:camera).permit(
-        :flash_type, :sd_card_slot, :network_interface, :camera_ip_address,
+        :flash_type, :partition_layout, :sd_card_slot, :network_interface, :camera_ip_address,
         :server_ip_address, :firmware_version, :sd_card_slot, :camera_mac_address
       )
     end

app/helpers/installation_helper.rb

@@ -90,10 +90,20 @@ def firmware_backup(c)
 
   def flashing_everything(c)
     fw_filename = Firmware.filename_for(soc_model: c.soc.model_downcase, flash_type: c.flash_type_type,
-                                        release: c.firmware_version, size: c.flash_size)
+                                        release: c.firmware_version, size: c.flash_size,
+                                        layout: c.layout_size)
     # The full image is exactly the size it claims on NOR and page-aligned by
     # construction on NAND, so ${filesize} is always a safe write length here --
     # unlike the u-boot-only block below, where the binary is neither.
+    #
+    # And on NOR that size is the chip's, whatever partition layout is going
+    # inside it, so the erase below spans the whole part. It used to span the
+    # size of the layout instead, which is the same number for every
+    # combination the menu could produce until it grew a second field -- and
+    # then, for an 8MB layout on a 16MB chip, left the top half of the flash
+    # untouched. The overlay ends `-(rootfs_data)` and so runs to the end of
+    # the device: a jffs2 that survives up there is mounted on the next boot
+    # and the camera comes back exactly as broken as it went in.
     write_size = '${filesize}'
     text = []
     text << do_not_copy_paste

app/helpers/selects_helper.rb

@@ -21,6 +21,13 @@ def list_of_flash_type_sizes_for_select
     end
   end
 
+  # Both layouts, always. Which of them a chip can hold is narrowed on the page,
+  # beside the rule that narrows the editions -- an 8MB part cannot wear the
+  # 16MB layout, whose rootfs partition ends at 0xD50000.
+  def list_of_partition_layouts_for_select
+    Camera::PARTITION_LAYOUT.map { |layout| [t("flash_layout.#{layout}"), layout] }
+  end
+
   # Every SoC used to be offered lite, ultimate and fabricator whatever upstream
   # built. The list comes from the release index now -- the union across flash
   # types, because the flash type is chosen in the same form without a round

app/models/camera.rb

@@ -10,6 +10,21 @@ class Camera
   # never has. `neo` is here because it does exist, for seven boards.
   FW_VERSION = %w[lite ultimate neo].freeze
   FLASH_CHIP = %w[nor8m nor16m nor32m nand].freeze
+
+  # How the chip is carved up, which is a different question from how big it
+  # is. Every OpenIPC bootloader carries both mtdpartsnor8m and mtdpartsnor16m
+  # and either can be run on a part large enough to hold it, so a 16MB chip can
+  # perfectly well wear the 8MB layout -- and one flashed from the 8MB image
+  # already does.
+  #
+  # The two were a single menu entry until a camera turned up that could not be
+  # revived by a full reflash. Its chip was 16MB and the 8MB entry had been
+  # chosen, so the instructions erased 0x0..0x800000 and stopped. Every layout
+  # here ends `-(rootfs_data)`, meaning "to the end of the device", so the
+  # overlay ran to 0x1000000 and the half of it above the erase survived intact.
+  # /init mounts jffs2 and only reformats when that mount fails, so the old,
+  # broken overlay came back every time.
+  PARTITION_LAYOUT = %w[nor8m nor16m].freeze
   NET_IFACE = %w[eth wifi both].freeze
   SD_CARD = %w[nosd sd].freeze
 
@@ -36,6 +51,7 @@ class Camera
   attr_accessor :soc_id, :needs_instruction, :flash_type, :sd_card_slot,
                 :network_interface, :camera_ip_address, :server_ip_address,
                 :firmware_version, :camera_mac_address, :soc, :backup_filename
+  attr_writer :partition_layout
 
   validates :soc_id, presence: true
   validates :flash_type, presence: true
@@ -185,12 +201,56 @@ def firmware_version_name
                                       default: firmware_version.to_s.capitalize)
   end
 
+  # Which mtdparts this camera is being given. Defaults to the one that matches
+  # the chip, so a visitor who never opens the second menu gets exactly what
+  # this page has always produced, and refuses a layout the chip cannot hold --
+  # the 16MB one ends at 0xD50000, which is past the end of an 8MB part.
+  #
+  # Unrecognised is the same as unset. Like every other field here it can arrive
+  # from a query string, and nothing calls valid? on a Camera.
+  def partition_layout
+    return 'nand' if nand?
+    return default_partition_layout unless @partition_layout.in?(PARTITION_LAYOUT)
+    return default_partition_layout unless layout_fits_chip?(@partition_layout)
+
+    @partition_layout
+  end
+
+  def default_partition_layout
+    return 'nand' if nand?
+
+    flash_size <= 8 ? 'nor8m' : 'nor16m'
+  end
+
+  # The 8MB layout fits anything; the 16MB one needs a 16MB part.
+  def layout_fits_chip?(layout)
+    layout.eql?('nor8m') || flash_size >= 16
+  end
+
+  def layout_size
+    partition_layout.eql?('nor8m') ? 8 : 16
+  end
+
+  # `nand` is not in PARTITION_LAYOUT and cannot be chosen: NAND has one layout,
+  # mtdpartsubi, and the menu is hidden for it. It is what partition_layout
+  # answers there so that the name doubles as the suffix of the bootloader
+  # macros -- `uknand`, `urnand`, `setnand`, and `uknor8m` and friends on NOR.
+  # That is also why a 32MB part has always been told to `run setnor16m`: there
+  # is no mtdpartsnor32m in any bootloader upstream ships.
+  def partition_layout_name
+    I18n.t("flash_layout.#{partition_layout}")
+  end
+
   # The NOR numbers come from FlashLayout, which reads them off the bootloader
   # environment. They used to be spelled out here keyed on firmware_version,
   # which agreed with the bootloader only for 8MB+Lite and 16MB+Ultimate; see
   # FlashLayout for what that cost on 16MB.
+  #
+  # Keyed on the layout, not on the chip. The two agree for every combination
+  # the menu offered before it grew a second field, and the whole point of the
+  # second field is the ones where they do not.
   def nor_layout
-    FlashLayout.nor(flash_size)
+    FlashLayout.nor(layout_size)
   end
 
   def kernel_max_size
@@ -251,6 +311,9 @@ def permalink
       '&sip=', server_ip_address,
       '&net=', network_interface,
       '&rom=', flash_type,
+      # The layout is written whether or not it differs from the chip's own, so
+      # a link says what it means rather than leaning on today's default.
+      '&part=', partition_layout,
       # `ver`, not `var`. This emitted `var` while show has always read `ver`,
       # so the edition was the one field the permanent link dropped: reopening
       # a link for Ultimate on a 32MB chip came back as Lite. show still